René Scharfe <l....@web.de> writes: > How about this instead? > > -- >8 -- > Subject: [PATCH] fsmonitor: use internal argv_array of struct child_process > > Avoid magic array sizes and indexes by constructing the fsmonitor > command line using the embedded argv_array of the child_process. The > resulting code is shorter and easier to extend. > > Getting rid of the snprintf() calls is a bonus -- even though the > buffers were big enough here to avoid truncation -- as it makes auditing > the remaining callers easier. > ... > - if (!(argv[0] = core_fsmonitor)) > + if (!core_fsmonitor) > return -1; > > - snprintf(ver, sizeof(ver), "%d", version);
I really like this change, as this exact line used to take sizeof(version) instead of sizeof(ver), which has been corrected recently. > - snprintf(date, sizeof(date), "%" PRIuMAX, (uintmax_t)last_update); > - argv[1] = ver; > - argv[2] = date; > - argv[3] = NULL; > - cp.argv = argv; > + argv_array_push(&cp.args, core_fsmonitor); > + argv_array_pushf(&cp.args, "%d", version); If it were written like this from the beginning, such a bug would never have happened ;-) > + argv_array_pushf(&cp.args, "%" PRIuMAX, (uintmax_t)last_update); > cp.use_shell = 1; > cp.dir = get_git_work_tree();
