On Thu, Mar 23, 2017 at 8:25 AM, Ramsay Jones
<[email protected]> wrote:
>> +static struct dir_entry *hash_dir_entry_with_parent_and_prefix(
>> +     struct index_state *istate,
>> +     struct dir_entry *parent,
>> +     struct strbuf *prefix)
>> +{
>> +     struct dir_entry *dir;
>> +     unsigned int hash;
>> +     int lock_nr;
>> +
>> +     /*
>> +      * Either we have a parent directory and path with slash(es)
>> +      * or the directory is an immediate child of the root directory.
>> +      */
>> +     assert((parent != NULL) ^ (strchr(prefix->buf, '/') == 0));
>
> sparse complains about 'Using plain integer as a NULL pointer', (the
> return from strchr() is NULL, if '/' is not found) so maybe:
>
> +       assert((parent != NULL) ^ (strchr(prefix->buf, '/') == NULL));
>

Also this seems part of the actual shipped code, not testing code.
In that case we prefer

    if (<condition>)
        die("BUG: <description>");

This is because asserts may be omitted by the compiler,
when compiled with NDEBUG.

However it is preferable to have these error checking code
in the shipped product, as opposed to users running into
that edge case and reporting an error which is totally non-obvious.

Thanks,
Stefan

Reply via email to