| Message ID | 20190820034536.13071-3-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes: > read_author_script reads name, email and author date from the author > script. However, it does not check if the arguments are NULL. Adding > NULL checks will allow us to selectively get the required value,... I had a hard time understanding the argument here without knowing why I had trouble, and I think I figured it out. What you wrote may not be incorrect per-se, but the logic is backwards. The function has been about reading all three, and it always took and required the callers to pass three valid pointers to locations to store these three. It did not check for NULL; passing NULL was simply a bug in the caller who deserved a segfault. This series, however, wants to allow new callers of the function to selectively read some among the three and ignore the rest, and you happened to choose "pass NULL for an uninteresting field" as the new calling convention. That choice is where "checking for NULL" comes in. In other words, "checking for NULL" is merely an implementation detail for a more important change this patch brings in: We now can read some and ignore the rest, while requiring that the input file is well formed even for the fields we do not care about. sequencer: allow callers of read_author_script() to ignore fields The current callers of the read_author_script() function read name, email and date from the author script. Allow callers to signal that they are not interested in some among these three fields by passing NULL. Note that fields that are ignored still must exist and be formatted correctly in the author script. or something like that, perhaps.
diff --git a/sequencer.c b/sequencer.c index 34ebf8ed94..30d77c2682 100644 --- a/sequencer.c +++ b/sequencer.c @@ -824,9 +824,19 @@ int read_author_script(const char *path, char **name, char **email, char **date, error(_("missing 'GIT_AUTHOR_DATE'")); if (date_i < 0 || email_i < 0 || date_i < 0 || err) goto finish; - *name = kv.items[name_i].util; - *email = kv.items[email_i].util; - *date = kv.items[date_i].util; + + if (name) + *name = kv.items[name_i].util; + else + free(kv.items[name_i].util); + if (email) + *email = kv.items[email_i].util; + else + free(kv.items[email_i].util); + if (date) + *date = kv.items[date_i].util; + else + free(kv.items[date_i].util); retval = 0; finish: string_list_clear(&kv, !!retval);
read_author_script reads name, email and author date from the author script. However, it does not check if the arguments are NULL. Adding NULL checks will allow us to selectively get the required value, for example: char *date; if (read_author_script(_path_, NULL, NULL, &date, _int_)) die(_("failed to read author date")); /* needs to be free()'d */ return date; Add NULL checks for better control over the information retrieved. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> --- sequencer.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)