Junio C Hamano <gitster@xxxxxxxxx> 于2021年7月25日周日 上午1:41写道：
>
> ZheNing Hu <adlternative@xxxxxxxxx> 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.
>
OK. I know it now, I will do all of them.
> 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.
>
Thanks for clarification, I understand the difference between them now.
> 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.
>
Yeah, as abspath.c:137, `ssize_t len` used to distinguish situations
if an error is
returned from strbuf_readlink().
ssize_t len;
...
len = strbuf_readlink(&symlink, resolved->buf,
st.st_size);
if (len < 0) {
if (flags & REALPATH_DIE_ON_ERROR)
die_errno("Invalid symlink '%s'",
resolved->buf);
else
goto error_out;
}
...
Or just read() and write(), they return is ssize_t too,
which can return -1 and set errno when an exception occurs.
Thanks.
--
ZheNing Hu
