Skip to content

Commit bb8bab1

Browse files
kurokkurok
committed
refactor: address code review feedback
- Extract select_default_fields() to deduplicate field selection logic shared between -F and -J/-j default handling (jiegec review) - Escape JSON keys via json_print_key() helper (jiegec review) - Add json_print_char() for single-char fields like access and lock, avoiding unnecessary char[2] array construction (jiegec review)
1 parent 34d6b1b commit bb8bab1

File tree

2 files changed

+96
-106
lines changed

2 files changed

+96
-106
lines changed

‎src/main.c‎

Lines changed: 55 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,58 @@ static int GetOpt(struct lsof_context *ctx, int ct, char *opt[], char *rules,
4747
int *err);
4848
static char *sv_fmt_str(struct lsof_context *ctx, char *f);
4949

50+
/*
51+
* select_default_fields() - enable the default set of FieldSel entries
52+
*
53+
* Shared by -F (with no field chars) and -J/-j (when no -F given).
54+
*/
55+
static void select_default_fields(void) {
56+
int i;
57+
for (i = 0; FieldSel[i].nm; i++) {
58+
59+
#if !defined(HASPPID)
60+
if (FieldSel[i].id == LSOF_FID_PPID)
61+
continue;
62+
#endif /* !defined(HASPPID) */
63+
64+
#if !defined(HASTASKS)
65+
if (FieldSel[i].id == LSOF_FID_TCMD)
66+
continue;
67+
#endif /* !defined(HASTASKS) */
68+
69+
#if !defined(HASFSTRUCT)
70+
if (FieldSel[i].id == LSOF_FID_CT || FieldSel[i].id == LSOF_FID_FA ||
71+
FieldSel[i].id == LSOF_FID_FG || FieldSel[i].id == LSOF_FID_NI)
72+
continue;
73+
#endif /* !defined(HASFSTRUCT) */
74+
75+
#if defined(HASSELINUX)
76+
if ((FieldSel[i].id == LSOF_FID_CNTX) && !CntxStatus)
77+
continue;
78+
#else /* !defined(HASSELINUX) */
79+
if (FieldSel[i].id == LSOF_FID_CNTX)
80+
continue;
81+
#endif /* !defined(HASSELINUX) */
82+
83+
if (FieldSel[i].id == LSOF_FID_RDEV)
84+
continue; /* for compatibility */
85+
86+
#if !defined(HASTASKS)
87+
if (FieldSel[i].id == LSOF_FID_TID)
88+
continue;
89+
#endif /* !defined(HASTASKS) */
90+
91+
#if !defined(HASZONES)
92+
if (FieldSel[i].id == LSOF_FID_ZONE)
93+
continue;
94+
#endif /* !defined(HASZONES) */
95+
96+
FieldSel[i].st = 1;
97+
if (FieldSel[i].opt && FieldSel[i].ov)
98+
*(FieldSel[i].opt) |= FieldSel[i].ov;
99+
}
100+
}
101+
50102
/*
51103
* main() - main function for lsof
52104
*/
@@ -429,51 +481,7 @@ int main(int argc, char *argv[]) {
429481
} else if (*GOv == '0')
430482
Terminator = '\0';
431483
}
432-
for (i = 0; FieldSel[i].nm; i++) {
433-
434-
#if !defined(HASPPID)
435-
if (FieldSel[i].id == LSOF_FID_PPID)
436-
continue;
437-
#endif /* !defined(HASPPID) */
438-
439-
#if !defined(HASTASKS)
440-
if (FieldSel[i].id == LSOF_FID_TCMD)
441-
continue;
442-
#endif /* !defined(HASTASKS) */
443-
444-
#if !defined(HASFSTRUCT)
445-
if (FieldSel[i].id == LSOF_FID_CT ||
446-
FieldSel[i].id == LSOF_FID_FA ||
447-
FieldSel[i].id == LSOF_FID_FG ||
448-
FieldSel[i].id == LSOF_FID_NI)
449-
continue;
450-
#endif /* !defined(HASFSTRUCT) */
451-
452-
#if defined(HASSELINUX)
453-
if ((FieldSel[i].id == LSOF_FID_CNTX) && !CntxStatus)
454-
continue;
455-
#else /* !defined(HASSELINUX) */
456-
if (FieldSel[i].id == LSOF_FID_CNTX)
457-
continue;
458-
#endif /* !defined(HASSELINUX) */
459-
460-
if (FieldSel[i].id == LSOF_FID_RDEV)
461-
continue; /* for compatibility */
462-
463-
#if !defined(HASTASKS)
464-
if (FieldSel[i].id == LSOF_FID_TID)
465-
continue;
466-
#endif /* !defined(HASTASKS) */
467-
468-
#if !defined(HASZONES)
469-
if (FieldSel[i].id == LSOF_FID_ZONE)
470-
continue;
471-
#endif /* !defined(HASZONES) */
472-
473-
FieldSel[i].st = 1;
474-
if (FieldSel[i].opt && FieldSel[i].ov)
475-
*(FieldSel[i].opt) |= FieldSel[i].ov;
476-
}
484+
select_default_fields();
477485

478486
#if defined(HASFSTRUCT)
479487
Ffield = FsvFlagX = 1;
@@ -1166,42 +1174,8 @@ int main(int argc, char *argv[]) {
11661174
break;
11671175
}
11681176
}
1169-
if (!has_fields) {
1170-
for (i = 0; FieldSel[i].nm; i++) {
1171-
#if !defined(HASPPID)
1172-
if (FieldSel[i].id == LSOF_FID_PPID)
1173-
continue;
1174-
#endif
1175-
#if !defined(HASTASKS)
1176-
if (FieldSel[i].id == LSOF_FID_TCMD ||
1177-
FieldSel[i].id == LSOF_FID_TID)
1178-
continue;
1179-
#endif
1180-
#if !defined(HASFSTRUCT)
1181-
if (FieldSel[i].id == LSOF_FID_CT ||
1182-
FieldSel[i].id == LSOF_FID_FA ||
1183-
FieldSel[i].id == LSOF_FID_FG ||
1184-
FieldSel[i].id == LSOF_FID_NI)
1185-
continue;
1186-
#endif
1187-
#if defined(HASSELINUX)
1188-
if ((FieldSel[i].id == LSOF_FID_CNTX) && !CntxStatus)
1189-
continue;
1190-
#else
1191-
if (FieldSel[i].id == LSOF_FID_CNTX)
1192-
continue;
1193-
#endif
1194-
if (FieldSel[i].id == LSOF_FID_RDEV)
1195-
continue;
1196-
#if !defined(HASZONES)
1197-
if (FieldSel[i].id == LSOF_FID_ZONE)
1198-
continue;
1199-
#endif
1200-
FieldSel[i].st = 1;
1201-
if (FieldSel[i].opt && FieldSel[i].ov)
1202-
*(FieldSel[i].opt) |= FieldSel[i].ov;
1203-
}
1204-
}
1177+
if (!has_fields)
1178+
select_default_fields();
12051179
}
12061180

12071181
if (DChelp || err || Fhelp || fh || version)

‎src/print.c‎

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,41 +135,57 @@ static void json_puts_escaped(const char *s) {
135135
}
136136
}
137137

138-
static void json_print_str(int *sep, const char *key, const char *val) {
138+
/*
139+
* json_print_key() - print comma separator and escaped JSON key
140+
*/
141+
static void json_print_key(int *sep, const char *key) {
139142
if (*sep)
140143
putchar(',');
141-
printf("\"%s\":\"", key);
142-
json_puts_escaped(val);
143144
putchar('"');
145+
json_puts_escaped(key);
146+
printf("\":");
144147
*sep = 1;
145148
}
146149

150+
static void json_print_str(int *sep, const char *key, const char *val) {
151+
json_print_key(sep, key);
152+
putchar('"');
153+
json_puts_escaped(val);
154+
putchar('"');
155+
}
156+
157+
static void json_print_char(int *sep, const char *key, char val) {
158+
json_print_key(sep, key);
159+
putchar('"');
160+
if (val == '"')
161+
fputs("\\\"", stdout);
162+
else if (val == '\\')
163+
fputs("\\\\", stdout);
164+
else if (val < 0x20)
165+
printf("\\u%04x", (unsigned int)(unsigned char)val);
166+
else
167+
putchar(val);
168+
putchar('"');
169+
}
170+
147171
static void json_print_int(int *sep, const char *key, int val) {
148-
if (*sep)
149-
putchar(',');
150-
printf("\"%s\":%d", key, val);
151-
*sep = 1;
172+
json_print_key(sep, key);
173+
printf("%d", val);
152174
}
153175

154176
static void json_print_uint64_str(int *sep, const char *key, uint64_t val) {
155-
if (*sep)
156-
putchar(',');
157-
printf("\"%s\":\"%" PRIu64 "\"", key, val);
158-
*sep = 1;
177+
json_print_key(sep, key);
178+
printf("\"%" PRIu64 "\"", val);
159179
}
160180

161181
static void json_print_long(int *sep, const char *key, long val) {
162-
if (*sep)
163-
putchar(',');
164-
printf("\"%s\":%ld", key, val);
165-
*sep = 1;
182+
json_print_key(sep, key);
183+
printf("%ld", val);
166184
}
167185

168186
static void json_print_ulong(int *sep, const char *key, unsigned long val) {
169-
if (*sep)
170-
putchar(',');
171-
printf("\"%s\":%lu", key, val);
172-
*sep = 1;
187+
json_print_key(sep, key);
188+
printf("%lu", val);
173189
}
174190

175191
#if !defined(HASNORPC_H)
@@ -1902,14 +1918,14 @@ static void json_print_file(struct lsof_context *ctx, int *sep) {
19021918
}
19031919
}
19041920
if (FieldSel[LSOF_FIX_ACCESS].st) {
1905-
char a[2] = {access_to_char(Lf->access), '\0'};
1906-
if (a[0] != ' ')
1907-
json_print_str(sep, "access", a);
1921+
char a = access_to_char(Lf->access);
1922+
if (a != ' ')
1923+
json_print_char(sep, "access", a);
19081924
}
19091925
if (FieldSel[LSOF_FIX_LOCK].st) {
1910-
char l[2] = {lock_to_char(Lf->lock), '\0'};
1911-
if (l[0] != ' ')
1912-
json_print_str(sep, "lock", l);
1926+
char l = lock_to_char(Lf->lock);
1927+
if (l != ' ')
1928+
json_print_char(sep, "lock", l);
19131929
}
19141930
if (FieldSel[LSOF_FIX_TYPE].st && Lf->type != LSOF_FILE_NONE) {
19151931
file_type_to_string(Lf->type, Lf->unknown_file_type_number, type,

0 commit comments

Comments
 (0)