Re: [PATCH v2 2/5] [GSOC] ref-filter: add %(raw) atom

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

ZheNing Hu <adlternative@gmail.com> writes:

>> It may make sense to
>>
>>  * Turn atom_value.s_size field into ssize_t instead of size_t
>>
>
> Will the conversion of size_t and ssize_t break -Wsign-conversion?
> Although there is a lot of code in Git that has broken it, but I am not
> sure if it is wise to use ssize_t here.
>
>>  * Rewrite (v->s_size != -1) comparison to (v->s_size < 0)
>>
>
> For size_t, this scheme is wrong.
>
>>
>> Optionally we could introduce #define ATOM_SIZE_UNSPECIFIED (-1) and
>> use it to initialize .s_size in ATOM_VALUE_INIT, but if we are just
>> going to test "is it negative, then it is not given", then it probably
>> is not needed.
>>
>
> It seems that this is the only method left. Although I think
> ATOM_SIZE_UNSPECIFIED
> is not very useful becasue we already have ATOM_VALUE_INIT.

Sorry, but I think you misread what I wrote.

These three were not offered as "you can do one of these three, pick
one you like" choices.  I meant to say "I think it makes sense to do
all these three things, but the last one is optional, doing only the
first two may be good enough".  As the second one requires that the
first is done, of course, doing only the second one would not make
sense.

Also, you seem to have missed the distinction between _INIT and
_UNSPECIFIED.  You can initialize something to (1) a reasonable
default value, or (2) unusable value that you can detect at runtime
and notice that it was not set.  If you called something to
FOO_INIT, your readers cannot tell which one it is, but if you call
it FOO_UNSPECIFIED, it is clear it is the latter case.

In many places, we do use ssize_t for "normally this is size, but we
can express exception cases (like errors) by storing negative value"
in our codebase (grep for it), and I think the member in question is
prime candidate for such use.