Skip to content

Commit c1189ca

Browse files
peffgitster
peff
authored and
gitster
committed
refactor argv_array into generic code
The submodule code recently grew generic code to build a dynamic argv array. Many other parts of the code can reuse this, too, so let's make it generically available. There are two enhancements not found in the original code: 1. We now handle the NULL-termination invariant properly, even when no strings have been pushed (before, you could have an empty, NULL argv). This was not a problem for the submodule code, which always pushed at least one argument, but was not sufficiently safe for generic code. 2. There is a formatted variant of the "push" function. This is a convenience function which was not needed by the submodule code, but will make it easier to port other users to the new code. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 7878b07 commit c1189ca

5 files changed

Lines changed: 125 additions & 35 deletions

File tree

‎Documentation/technical/api-argv-array.txt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
argv-array API
2+
==============
3+
4+
The argv-array API allows one to dynamically build and store
5+
NULL-terminated lists. An argv-array maintains the invariant that the
6+
`argv` member always points to a non-NULL array, and that the array is
7+
always NULL-terminated at the element pointed to by `argv[argc]`. This
8+
makes the result suitable for passing to functions expecting to receive
9+
argv from main(), or the link:api-run-command.html[run-command API].
10+
11+
The link:api-string-list.html[string-list API] is similar, but cannot be
12+
used for these purposes; instead of storing a straight string pointer,
13+
it contains an item structure with a `util` field that is not compatible
14+
with the traditional argv interface.
15+
16+
Each `argv_array` manages its own memory. Any strings pushed into the
17+
array are duplicated, and all memory is freed by argv_array_clear().
18+
19+
Data Structures
20+
---------------
21+
22+
`struct argv_array`::
23+
24+
A single array. This should be initialized by assignment from
25+
`ARGV_ARRAY_INIT`, or by calling `argv_array_init`. The `argv`
26+
member contains the actual array; the `argc` member contains the
27+
number of elements in the array, not including the terminating
28+
NULL.
29+
30+
Functions
31+
---------
32+
33+
`argv_array_init`::
34+
Initialize an array. This is no different than assigning from
35+
`ARGV_ARRAY_INIT`.
36+
37+
`argv_array_push`::
38+
Push a copy of a string onto the end of the array.
39+
40+
`argv_array_pushf`::
41+
Format a string and push it onto the end of the array. This is a
42+
convenience wrapper combining `strbuf_addf` and `argv_array_push`.
43+
44+
`argv_array_clear`::
45+
Free all memory associated with the array and return it to the
46+
initial, empty state.

‎Makefile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,7 @@ VCSSVN_LIB=vcs-svn/lib.a
497497

498498
LIB_H += advice.h
499499
LIB_H += archive.h
500+
LIB_H += argv-array.h
500501
LIB_H += attr.h
501502
LIB_H += blob.h
502503
LIB_H += builtin.h
@@ -575,6 +576,7 @@ LIB_OBJS += alloc.o
575576
LIB_OBJS += archive.o
576577
LIB_OBJS += archive-tar.o
577578
LIB_OBJS += archive-zip.o
579+
LIB_OBJS += argv-array.o
578580
LIB_OBJS += attr.o
579581
LIB_OBJS += base85.o
580582
LIB_OBJS += bisect.o

‎argv-array.c‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
#include "cache.h"
2+
#include "argv-array.h"
3+
#include "strbuf.h"
4+
5+
static const char *empty_argv_storage = NULL;
6+
const char **empty_argv = &empty_argv_storage;
7+
8+
void argv_array_init(struct argv_array *array)
9+
{
10+
array->argv = empty_argv;
11+
array->argc = 0;
12+
array->alloc = 0;
13+
}
14+
15+
static void argv_array_push_nodup(struct argv_array *array, const char *value)
16+
{
17+
if (array->argv == empty_argv)
18+
array->argv = NULL;
19+
20+
ALLOC_GROW(array->argv, array->argc + 2, array->alloc);
21+
array->argv[array->argc++] = value;
22+
array->argv[array->argc] = NULL;
23+
}
24+
25+
void argv_array_push(struct argv_array *array, const char *value)
26+
{
27+
argv_array_push_nodup(array, xstrdup(value));
28+
}
29+
30+
void argv_array_pushf(struct argv_array *array, const char *fmt, ...)
31+
{
32+
va_list ap;
33+
struct strbuf v = STRBUF_INIT;
34+
35+
va_start(ap, fmt);
36+
strbuf_vaddf(&v, fmt, ap);
37+
va_end(ap);
38+
39+
argv_array_push_nodup(array, strbuf_detach(&v, NULL));
40+
}
41+
42+
void argv_array_clear(struct argv_array *array)
43+
{
44+
if (array->argv != empty_argv) {
45+
int i;
46+
for (i = 0; i < array->argc; i++)
47+
free((char **)array->argv[i]);
48+
free(array->argv);
49+
}
50+
argv_array_init(array);
51+
}

‎argv-array.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#ifndef ARGV_ARRAY_H
2+
#define ARGV_ARRAY_H
3+
4+
extern const char **empty_argv;
5+
6+
struct argv_array {
7+
const char **argv;
8+
int argc;
9+
int alloc;
10+
};
11+
12+
#define ARGV_ARRAY_INIT { empty_argv, 0, 0 }
13+
14+
void argv_array_init(struct argv_array *);
15+
void argv_array_push(struct argv_array *, const char *);
16+
__attribute__((format (printf,2,3)))
17+
void argv_array_pushf(struct argv_array *, const char *fmt, ...);
18+
void argv_array_clear(struct argv_array *);
19+
20+
#endif /* ARGV_ARRAY_H */

‎submodule.c‎

Lines changed: 6 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "refs.h"
1010
#include "string-list.h"
1111
#include "sha1-array.h"
12+
#include "argv-array.h"
1213

1314
static struct string_list config_name_for_path;
1415
static struct string_list config_fetch_recurse_submodules_for_name;
@@ -388,52 +389,22 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])
388389
sha1_array_append(&ref_tips_after_fetch, new_sha1);
389390
}
390391

391-
struct argv_array {
392-
const char **argv;
393-
unsigned int argc;
394-
unsigned int alloc;
395-
};
396-
397-
static void init_argv(struct argv_array *array)
398-
{
399-
array->argv = NULL;
400-
array->argc = 0;
401-
array->alloc = 0;
402-
}
403-
404-
static void push_argv(struct argv_array *array, const char *value)
405-
{
406-
ALLOC_GROW(array->argv, array->argc + 2, array->alloc);
407-
array->argv[array->argc++] = xstrdup(value);
408-
array->argv[array->argc] = NULL;
409-
}
410-
411-
static void clear_argv(struct argv_array *array)
412-
{
413-
int i;
414-
for (i = 0; i < array->argc; i++)
415-
free((char **)array->argv[i]);
416-
free(array->argv);
417-
init_argv(array);
418-
}
419-
420392
static void add_sha1_to_argv(const unsigned char sha1[20], void *data)
421393
{
422-
push_argv(data, sha1_to_hex(sha1));
394+
argv_array_push(data, sha1_to_hex(sha1));
423395
}
424396

425397
static void calculate_changed_submodule_paths(void)
426398
{
427399
struct rev_info rev;
428400
struct commit *commit;
429-
struct argv_array argv;
401+
struct argv_array argv = ARGV_ARRAY_INIT;
430402

431403
init_revisions(&rev, NULL);
432-
init_argv(&argv);
433-
push_argv(&argv, "--"); /* argv[0] program name */
404+
argv_array_push(&argv, "--"); /* argv[0] program name */
434405
sha1_array_for_each_unique(&ref_tips_after_fetch,
435406
add_sha1_to_argv, &argv);
436-
push_argv(&argv, "--not");
407+
argv_array_push(&argv, "--not");
437408
sha1_array_for_each_unique(&ref_tips_before_fetch,
438409
add_sha1_to_argv, &argv);
439410
setup_revisions(argv.argc, argv.argv, &rev, NULL);
@@ -460,7 +431,7 @@ static void calculate_changed_submodule_paths(void)
460431
}
461432
}
462433

463-
clear_argv(&argv);
434+
argv_array_clear(&argv);
464435
sha1_array_clear(&ref_tips_before_fetch);
465436
sha1_array_clear(&ref_tips_after_fetch);
466437
initialized_fetch_ref_tips = 0;

0 commit comments

Comments
 (0)