Skip to content

Commit 8a98272

Browse files
twaughtwaugh
committed
Refactor ls.c and grep.c to use common option parsing
This change eliminates code duplication by centralizing common option parsing logic in patch_common.c, making both tools use the shared infrastructure that was previously unused. Changes: - Enhanced patch_common.c to support -I/-X options (include/exclude from file) - Refactored ls.c to use init_common_options(), parse_common_option(), cleanup_common_options(), add_common_long_options(), and get_common_short_options() - Refactored grep.c to use the same common option parsing functions - Removed ~70 lines of duplicate option parsing code between the two files - Added proper constants for option array sizes (MAX_COMMON_OPTIONS, MAX_TOOL_OPTIONS, MAX_TOTAL_OPTIONS) with runtime safety checks - Fixed unused variable warnings Benefits: - Common options are now handled consistently in one place - Easier to maintain and extend common functionality - Reduced code duplication and improved organization - Runtime protection against accidentally exceeding option array limits - All existing functionality preserved and tested Assisted-by: Cursor
1 parent b9380f0 commit 8a98272

4 files changed

Lines changed: 101 additions & 199 deletions

File tree

‎src/grep.c‎

Lines changed: 40 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -863,117 +863,65 @@ int run_grep_mode(int argc, char *argv[])
863863
int i;
864864
FILE *fp;
865865

866-
/* Reset global state for each invocation */
867-
global_line_offset = 0;
866+
/* Initialize common options */
867+
init_common_options();
868868

869869
setlocale(LC_TIME, "C");
870870

871871
while (1) {
872-
static struct option long_options[] = {
873-
{"help", 0, 0, 1000 + 'H'},
874-
{"version", 0, 0, 1000 + 'V'},
875-
{"line-number", 0, 0, 'n'},
876-
{"number-files", 0, 0, 'N'},
877-
{"with-filename", 0, 0, 'H'},
878-
{"no-filename", 0, 0, 'h'},
879-
{"strip-match", 1, 0, 'p'},
880-
{"include", 1, 0, 'i'},
881-
{"exclude", 1, 0, 'x'},
882-
{"verbose", 0, 0, 'v'},
883-
{"decompress", 0, 0, 'z'},
884-
{"extended-regexp", 0, 0, 'E'},
885-
{"file", 1, 0, 'f'},
886-
{"git-prefixes", 1, 0, 1000 + 'G'},
887-
{"strip", 1, 0, 1000 + 'S'},
888-
{"addprefix", 1, 0, 1000 + 'A'},
889-
{"addoldprefix", 1, 0, 1000 + 'O'},
890-
{"addnewprefix", 1, 0, 1000 + 'N'},
891-
{"output-matching", 1, 0, 1000 + 'M'},
892-
{"only-match", 1, 0, 1000 + 'm'},
893-
{"as-numbered-lines", 1, 0, 1000 + 'L'},
894-
/* Mode options (handled by patchfilter, but need to be recognized) */
895-
{"list", 0, 0, 1000 + 'l'},
896-
{"filter", 0, 0, 1000 + 'F'},
897-
{"grep", 0, 0, 1000 + 'g'},
898-
{0, 0, 0, 0}
899-
};
900-
901-
char *end;
902-
int c = getopt_long(argc, argv, "nNHhp:i:x:vzEf:", long_options, NULL);
872+
static struct option long_options[MAX_TOTAL_OPTIONS];
873+
int next_idx = 0;
874+
875+
/* Add common long options */
876+
add_common_long_options(long_options, &next_idx);
877+
878+
/* Add tool-specific long options */
879+
long_options[next_idx++] = (struct option){"help", 0, 0, 1000 + 'H'};
880+
long_options[next_idx++] = (struct option){"version", 0, 0, 1000 + 'V'};
881+
long_options[next_idx++] = (struct option){"extended-regexp", 0, 0, 'E'};
882+
long_options[next_idx++] = (struct option){"file", 1, 0, 'f'};
883+
long_options[next_idx++] = (struct option){"output-matching", 1, 0, 1000 + 'M'};
884+
long_options[next_idx++] = (struct option){"only-match", 1, 0, 1000 + 'm'};
885+
long_options[next_idx++] = (struct option){"as-numbered-lines", 1, 0, 1000 + 'L'};
886+
/* Mode options (handled by patchfilter, but need to be recognized) */
887+
long_options[next_idx++] = (struct option){"list", 0, 0, 1000 + 'l'};
888+
long_options[next_idx++] = (struct option){"filter", 0, 0, 1000 + 'F'};
889+
long_options[next_idx++] = (struct option){"grep", 0, 0, 1000 + 'g'};
890+
long_options[next_idx++] = (struct option){0, 0, 0, 0};
891+
892+
/* Safety check: ensure we haven't exceeded MAX_TOTAL_OPTIONS */
893+
if (next_idx > MAX_TOTAL_OPTIONS) {
894+
error(EXIT_FAILURE, 0, "Internal error: too many total options (%d > %d). "
895+
"Increase MAX_TOTAL_OPTIONS in patch_common.h", next_idx, MAX_TOTAL_OPTIONS);
896+
}
897+
898+
/* Combine common and tool-specific short options */
899+
char short_options[64];
900+
snprintf(short_options, sizeof(short_options), "%sEf:", get_common_short_options());
901+
902+
int c = getopt_long(argc, argv, short_options, long_options, NULL);
903903
if (c == -1)
904904
break;
905905

906+
/* Try common option parsing first */
907+
if (parse_common_option(c, optarg)) {
908+
continue;
909+
}
910+
911+
/* Handle tool-specific options */
906912
switch (c) {
907913
case 1000 + 'H':
908914
syntax(0);
909915
break;
910916
case 1000 + 'V':
911917
printf("grepdiff - patchutils version %s\n", VERSION);
912918
exit(0);
913-
case 'n':
914-
show_line_numbers = 1;
915-
break;
916-
case 'N':
917-
number_files = 1;
918-
break;
919-
case 'H':
920-
show_patch_names = 1;
921-
break;
922-
case 'h':
923-
show_patch_names = 0;
924-
break;
925-
case 'p':
926-
strip_components = strtoul(optarg, &end, 0);
927-
if (optarg == end)
928-
syntax(1);
929-
break;
930-
case 'i':
931-
patlist_add(&pat_include, optarg);
932-
break;
933-
case 'x':
934-
patlist_add(&pat_exclude, optarg);
935-
break;
936-
case 'v':
937-
verbose++;
938-
if (show_line_numbers && verbose > 1)
939-
number_files = 1;
940-
break;
941-
case 'z':
942-
unzip = 1;
943-
break;
944919
case 'E':
945920
extended_regexp = 1;
946921
break;
947922
case 'f':
948923
add_patterns_from_file(optarg);
949924
break;
950-
case 1000 + 'G':
951-
if (!strcmp(optarg, "strip")) {
952-
git_prefix_mode = GIT_PREFIX_STRIP;
953-
} else if (!strcmp(optarg, "keep")) {
954-
git_prefix_mode = GIT_PREFIX_KEEP;
955-
} else {
956-
error(EXIT_FAILURE, 0, "invalid argument to --git-prefixes: %s (expected 'strip' or 'keep')", optarg);
957-
}
958-
break;
959-
case 1000 + 'S':
960-
{
961-
char *end;
962-
strip_output_components = strtoul(optarg, &end, 0);
963-
if (optarg == end) {
964-
error(EXIT_FAILURE, 0, "invalid argument to --strip: %s", optarg);
965-
}
966-
}
967-
break;
968-
case 1000 + 'A':
969-
add_prefix = optarg;
970-
break;
971-
case 1000 + 'O':
972-
add_old_prefix = optarg;
973-
break;
974-
case 1000 + 'N':
975-
add_new_prefix = optarg;
976-
break;
977925
case 1000 + 'M':
978926
if (!strncmp(optarg, "file", 4)) {
979927
output_mode = OUTPUT_FILE;
@@ -1059,10 +1007,7 @@ int run_grep_mode(int argc, char *argv[])
10591007
}
10601008

10611009
/* Clean up */
1062-
if (pat_include)
1063-
patlist_free(&pat_include);
1064-
if (pat_exclude)
1065-
patlist_free(&pat_exclude);
1010+
cleanup_common_options();
10661011
if (grep_patterns) {
10671012
for (i = 0; i < num_grep_patterns; i++) {
10681013
regfree(&grep_patterns[i]);

‎src/ls.c‎

Lines changed: 40 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -336,48 +336,52 @@ int run_ls_mode(int argc, char *argv[])
336336
int i;
337337
FILE *fp;
338338

339-
/* Reset global line offset for each invocation */
340-
global_line_offset = 0;
339+
/* Initialize common options */
340+
init_common_options();
341341

342342
setlocale(LC_TIME, "C");
343343

344344
while (1) {
345-
static struct option long_options[] = {
346-
{"help", 0, 0, 1000 + 'H'},
347-
{"version", 0, 0, 1000 + 'V'},
348-
{"status", 0, 0, 's'},
349-
{"line-number", 0, 0, 'n'},
350-
{"number-files", 0, 0, 'N'},
351-
{"with-filename", 0, 0, 'H'},
352-
{"no-filename", 0, 0, 'h'},
353-
{"empty-files-as-absent", 0, 0, 'E'},
354-
{"strip-match", 1, 0, 'p'},
355-
{"include", 1, 0, 'i'},
356-
{"exclude", 1, 0, 'x'},
357-
{"include-from-file", 1, 0, 'I'},
358-
{"exclude-from-file", 1, 0, 'X'},
359-
{"files", 1, 0, 'F'},
360-
{"verbose", 0, 0, 'v'},
361-
{"decompress", 0, 0, 'z'},
362-
{"git-prefixes", 1, 0, 1000 + 'G'},
363-
{"strip", 1, 0, 1000 + 'S'},
364-
{"addprefix", 1, 0, 1000 + 'A'},
365-
{"addoldprefix", 1, 0, 1000 + 'O'},
366-
{"addnewprefix", 1, 0, 1000 + 'N'},
367-
{"lines", 1, 0, 1000 + 'L'},
368-
{"hunks", 1, 0, '#'},
369-
/* Mode options (handled by patchfilter, but need to be recognized) */
370-
{"list", 0, 0, 1000 + 'l'},
371-
{"filter", 0, 0, 1000 + 'f'},
372-
{"grep", 0, 0, 1000 + 'g'},
373-
{0, 0, 0, 0}
374-
};
375-
376-
char *end;
377-
int c = getopt_long(argc, argv, "snNHhEp:i:x:I:X:F:vz#:", long_options, NULL);
345+
static struct option long_options[MAX_TOTAL_OPTIONS];
346+
int next_idx = 0;
347+
348+
/* Add common long options */
349+
add_common_long_options(long_options, &next_idx);
350+
351+
/* Add tool-specific long options */
352+
long_options[next_idx++] = (struct option){"help", 0, 0, 1000 + 'H'};
353+
long_options[next_idx++] = (struct option){"version", 0, 0, 1000 + 'V'};
354+
long_options[next_idx++] = (struct option){"status", 0, 0, 's'};
355+
long_options[next_idx++] = (struct option){"empty-files-as-absent", 0, 0, 'E'};
356+
long_options[next_idx++] = (struct option){"files", 1, 0, 'F'};
357+
long_options[next_idx++] = (struct option){"lines", 1, 0, 1000 + 'L'};
358+
long_options[next_idx++] = (struct option){"hunks", 1, 0, '#'};
359+
/* Mode options (handled by patchfilter, but need to be recognized) */
360+
long_options[next_idx++] = (struct option){"list", 0, 0, 1000 + 'l'};
361+
long_options[next_idx++] = (struct option){"filter", 0, 0, 1000 + 'f'};
362+
long_options[next_idx++] = (struct option){"grep", 0, 0, 1000 + 'g'};
363+
long_options[next_idx++] = (struct option){0, 0, 0, 0};
364+
365+
/* Safety check: ensure we haven't exceeded MAX_TOTAL_OPTIONS */
366+
if (next_idx > MAX_TOTAL_OPTIONS) {
367+
error(EXIT_FAILURE, 0, "Internal error: too many total options (%d > %d). "
368+
"Increase MAX_TOTAL_OPTIONS in patch_common.h", next_idx, MAX_TOTAL_OPTIONS);
369+
}
370+
371+
/* Combine common and tool-specific short options */
372+
char short_options[64];
373+
snprintf(short_options, sizeof(short_options), "%ssEF:#:", get_common_short_options());
374+
375+
int c = getopt_long(argc, argv, short_options, long_options, NULL);
378376
if (c == -1)
379377
break;
380378

379+
/* Try common option parsing first */
380+
if (parse_common_option(c, optarg)) {
381+
continue;
382+
}
383+
384+
/* Handle tool-specific options */
381385
switch (c) {
382386
case 1000 + 'H':
383387
syntax(0);
@@ -388,38 +392,9 @@ int run_ls_mode(int argc, char *argv[])
388392
case 's':
389393
show_status = 1;
390394
break;
391-
case 'n':
392-
show_line_numbers = 1;
393-
break;
394-
case 'N':
395-
number_files = 1;
396-
break;
397-
case 'H':
398-
show_patch_names = 1;
399-
break;
400-
case 'h':
401-
show_patch_names = 0;
402-
break;
403395
case 'E':
404396
empty_files_as_absent = 1;
405397
break;
406-
case 'p':
407-
strip_components = strtoul(optarg, &end, 0);
408-
if (optarg == end)
409-
syntax(1);
410-
break;
411-
case 'i':
412-
patlist_add(&pat_include, optarg);
413-
break;
414-
case 'x':
415-
patlist_add(&pat_exclude, optarg);
416-
break;
417-
case 'I':
418-
patlist_add_file(&pat_include, optarg);
419-
break;
420-
case 'X':
421-
patlist_add_file(&pat_exclude, optarg);
422-
break;
423398
case 'F':
424399
if (files)
425400
syntax(1);
@@ -429,41 +404,6 @@ int run_ls_mode(int argc, char *argv[])
429404
}
430405
parse_range(&files, optarg);
431406
break;
432-
case 'v':
433-
verbose++;
434-
if (show_line_numbers && verbose > 1)
435-
number_files = 1;
436-
break;
437-
case 'z':
438-
unzip = 1;
439-
break;
440-
case 1000 + 'G':
441-
if (!strcmp(optarg, "strip")) {
442-
git_prefix_mode = GIT_PREFIX_STRIP;
443-
} else if (!strcmp(optarg, "keep")) {
444-
git_prefix_mode = GIT_PREFIX_KEEP;
445-
} else {
446-
error(EXIT_FAILURE, 0, "invalid argument to --git-prefixes: %s (expected 'strip' or 'keep')", optarg);
447-
}
448-
break;
449-
case 1000 + 'S':
450-
{
451-
char *end;
452-
strip_output_components = strtoul(optarg, &end, 0);
453-
if (optarg == end) {
454-
error(EXIT_FAILURE, 0, "invalid argument to --strip: %s", optarg);
455-
}
456-
}
457-
break;
458-
case 1000 + 'A':
459-
add_prefix = optarg;
460-
break;
461-
case 1000 + 'O':
462-
add_old_prefix = optarg;
463-
break;
464-
case 1000 + 'N':
465-
add_new_prefix = optarg;
466-
break;
467407
case 1000 + 'L':
468408
if (lines)
469409
syntax(1);
@@ -522,10 +462,7 @@ int run_ls_mode(int argc, char *argv[])
522462
}
523463

524464
/* Clean up */
525-
if (pat_include)
526-
patlist_free(&pat_include);
527-
if (pat_exclude)
528-
patlist_free(&pat_exclude);
465+
cleanup_common_options();
529466
if (files) {
530467
struct range *r, *next;
531468
for (r = files; r; r = next) {

‎src/patch_common.c‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ int parse_common_option(int c, char *optarg)
136136
case 'x':
137137
patlist_add(&pat_exclude, optarg);
138138
return 1;
139+
case 'I':
140+
patlist_add_file(&pat_include, optarg);
141+
return 1;
142+
case 'X':
143+
patlist_add_file(&pat_exclude, optarg);
144+
return 1;
139145
case 'v':
140146
verbose++;
141147
if (show_line_numbers && verbose > 1)
@@ -207,12 +213,13 @@ void cleanup_common_options(void)
207213

208214
const char *get_common_short_options(void)
209215
{
210-
return "nNHhp:i:x:vz";
216+
return "nNHhp:i:x:I:X:vz";
211217
}
212218

213219
void add_common_long_options(struct option *options, int *next_index)
214220
{
215221
int idx = *next_index;
222+
int start_idx = idx;
216223

217224
options[idx++] = (struct option){"line-number", 0, 0, 'n'};
218225
options[idx++] = (struct option){"number-files", 0, 0, 'N'};
@@ -221,6 +228,8 @@ void add_common_long_options(struct option *options, int *next_index)
221228
options[idx++] = (struct option){"strip-match", 1, 0, 'p'};
222229
options[idx++] = (struct option){"include", 1, 0, 'i'};
223230
options[idx++] = (struct option){"exclude", 1, 0, 'x'};
231+
options[idx++] = (struct option){"include-from-file", 1, 0, 'I'};
232+
options[idx++] = (struct option){"exclude-from-file", 1, 0, 'X'};
224233
options[idx++] = (struct option){"verbose", 0, 0, 'v'};
225234
options[idx++] = (struct option){"decompress", 0, 0, 'z'};
226235
options[idx++] = (struct option){"git-prefixes", 1, 0, 1000 + 'G'};
@@ -229,5 +238,12 @@ void add_common_long_options(struct option *options, int *next_index)
229238
options[idx++] = (struct option){"addoldprefix", 1, 0, 1000 + 'O'};
230239
options[idx++] = (struct option){"addnewprefix", 1, 0, 1000 + 'N'};
231240

241+
/* Safety check: ensure we haven't exceeded MAX_COMMON_OPTIONS */
242+
if (idx - start_idx > MAX_COMMON_OPTIONS) {
243+
error(EXIT_FAILURE, 0, "Internal error: too many common options (%d > %d). "
244+
"Increase MAX_COMMON_OPTIONS in patch_common.h",
245+
idx - start_idx, MAX_COMMON_OPTIONS);
246+
}
247+
232248
*next_index = idx;
233249
}

0 commit comments

Comments
 (0)