Re: [PATCH v3 2/6] sequencer: add NULL checks under read_author_script

[Junio C Hamano <gitster@pobox.com>]

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.