Hi Kurt,

On 2026-04-23T13:12:14-0400, Kurt Hackenberg wrote:
> On Thu, Apr 23, 2026 at 14:03 +0200, Alejandro Colomar via Mutt-dev wrote:
> 
> > > +  } while ((res == (size_t) -1) && (errno == EINTR));
> > 
> > This cast is dangerous.  It's safer to compare to literal -1.  I know
> > you'll get a -Wsign-compare (part of -Wextra) diagnostic, but that's
> > something that GCC should fix.  I've been using -Wno-error=sign-compare
> > for some time, precisely for the false negatives.
> > 
> > If for some reason, the type of the cast doesn't match the type of the
> > variable, we'll get a bug.  On the other hand, non-casted literal -1
> > will magically convert to any wider unsigned type and do the right
> > thing.
> 
> Short answer: getrandom() returns ssize_t.

Using ssize_t would make it better.

> Long explanation:
> 
> I guess the danger is that, if size_t is unsigned and wider than int, the
> value's high bits will be 0?

No.  -1 is sign-extended to cover as many bits as the unsigned integer
to which it is compared.  The problems will come if the unsigned integer
is narrower than int, not wider than int.

Proof:

        $ cat u.c 
        unsigned long  u;

        _Static_assert(sizeof(u) == 8);
        _Static_assert(sizeof(int) == 4);

        int
        main(void)
        {
                u = 0xFFFFffffFFFFffff;

                if (u == -1)
                        return 42;

                return 0;
        }

        $ gcc -Wall u.c 
        $ ./a.out; echo $?
        42

> And the magic is a special little hack built
> into the compiler (and language?) that recognizes the idiom, and does
> something non-standard?

That's perfectly standard.

> I'd be reluctant to count on the magic. I guess you could do either of:
>     ~0ULL
>     ~((size_t)0)

*NEVER* do this.  This is really dangerous.  As much as the cast.
See the link I posted in my previous mail.  The only safe way to do this
is really with literal -1 without casts.

> But that's still hacking around the problem, which is mixing signed and
> unsigned. It's probably better not to mix them.
> 
> The return value -1 itself is an old hack. It combines two things, a length
> and an error flag, in one value. Valid lengths are positive or zero; any
> negative value cannot be a length, so a specific negative value is assigned
> a different meaning.

-1 as an error code is well-known, and works fine.  You can mix literal
-1 with any unsigned types wider than int, and it will do the right
thing, because of the "usual arithmetic conversions".

The problematic thing is "integer promotions" --which is why literal -1
can't be mixed with unsigned integers narrower than int--, not "usual
arithmetic conversions" --which are fine--.

> That can only work for signed types. In an unsigned type, all possible
> values are valid lengths, so unsigned types should not be used for that
> two-meanings hack.

Not true.  I certainly prefer signed integers for most stuff, but
unsigned types wider than int also work well.  And for example, the
ISO C standard has a few functions that return size_t with an error code
of -1.  For example, see mbrlen(3).

> If the length has to be unsigned, then the second meaning -- error flag --
> could be put into a separate boolean variable. Or, maybe the two-meanings
> value can be a plain old int, assuming Mutt won't ask for any huge amount of
> data. Of course, only Mutt code can be adjusted, not arguments and return
> values of system library functions.

Using a separate variable for an error code is weird, and probably
dangerous, because it requires more careful handling of the extra
variable.

And using 'int' is dangerous, because it's hard to guarantee that
there's no way to overflow it.

size_t and ssize_t are much more reasonable.

> But maybe there isn't a problem. The complete code is:
> 
>   size_t res;
> 
>   do
>   {
>     res = getrandom(random_bytes, length_requested, GRND_NONBLOCK);
>   } while ((res == (size_t) -1) && (errno == EINTR));
> 
> On FreeBSD, the man page for getrandom() says the return type is not size_t,
> but ssize_t. I've never heard of ssize_t, but I bet it's signed, and I bet
> it exists specifically to avoid this problem.

ssize_t is a POSIX type.  It even has a manual page.
Check ssize_t(3type).  It was first standardized in POSIX.1‐1990.
And yes, it's a signed type.  We could (should) certainly use it.

> If ssize_t exists everywhere and is guaranteed to be signed, and getrandom()
> everywhere returns ssize_t, then it should be fine to just use that type and
> compare it directly to -1, like this:
> 
>   ssize_t res;          /* signed */
> 
>   do
>   {
>     res = getrandom(random_bytes, length_requested, GRND_NONBLOCK);
>   } while ((res == -1) && (errno == EINTR));

Yup, this would be good (preferrable).

But it would still work fine if you change s/ssize_t/size_t/, just to be
clear.


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

Attachment: signature.asc
Description: PGP signature

Reply via email to