runit: tryshsgr.c: why not test whether gid_t is short by using "sizeof(short) == sizeof(gid_t)"

runit: tryshsgr.c: why not test whether gid_t is short by using "sizeof(short) == sizeof(gid_t)"

[Zhixu Liu]

Hi,

I'm trying to keep runit remaining available in gentoo's official portage tree,
see https://bugs.gentoo.org/938282 . I setup a github repo at
https://github.com/clan/runit/ for this purpose, following is what I'm doing
now:
1. fix all the compilation error(s)
2. eliminate the compilation warning(s) as much as possible

Now I 'm in stage 2, when fixing the warning of
-Wincompatible-pointer-types, see
https://github.com/clan/runit/blob/master/admin/runit-2.1.2/src/tryshsgr.c

As G. Pape has commented at
https://github.com/clan/runit/commit/5a7b8099ae638ce72abae2c4d8cac7ce33477522

> "tryshsgr" is specifically there to check whether the size of gid_t is short,
> it doesn't matter if it fails at compile time or run time. But with this
> change, it always succeeds, and so always size short is
> assumed, even though most systems should have unsigned int I guess.

At first, what Pape said is correct, I didn't fully understand the semantic
meaning of this check before fixing. I should use debian's patch
(0029-fix-ftbfs-with-gcc14.patch) which casts x by (gid_t *) instead of
changing from short x[4] to gid_t x[4] in my current patch (which is wrong).

But I'm just wondering why we don't use "sizeof(short) == sizeof(gid_t)" to do
the test which is more intuitive?

Part of the reason is that I found the test of the result depends on the
running user's supplement group config after I changed short to gid_t, which
is very confusing. If I run as root with many supplement groups, the test
fails, if running as another user w/o supplement groups, the test succeeds.
Of course, after the change from short to gid_t, the test logic or assumption
is wrong from the beginning.

Thanks.

Re: runit: tryshsgr.c: why not test whether gid_t is short by using "sizeof(short) == sizeof(gid_t)"

[Leah Neukirchen]

Zhixu Liu <zhixu.liu@gmail.com> writes:

> Hi,
>
> I'm trying to keep runit remaining available in gentoo's official portage tree,
> see https://bugs.gentoo.org/938282 . I setup a github repo at
> https://github.com/clan/runit/ for this purpose, following is what I'm doing
> now:
> 1. fix all the compilation error(s)
> 2. eliminate the compilation warning(s) as much as possible
>
> Now I 'm in stage 2, when fixing the warning of
> -Wincompatible-pointer-types, see
> https://github.com/clan/runit/blob/master/admin/runit-2.1.2/src/tryshsgr.c
>
> As G. Pape has commented at
> https://github.com/clan/runit/commit/5a7b8099ae638ce72abae2c4d8cac7ce33477522
>
>> "tryshsgr" is specifically there to check whether the size of gid_t is short,
>> it doesn't matter if it fails at compile time or run time. But with this
>> change, it always succeeds, and so always size short is
>> assumed, even though most systems should have unsigned int I guess.

As of runit.git 84457fd2 (2005-08-23), prot_gid is unused and all of
this is dead code that can be removed.  setgroups is used directly
with proper gid_t types.

-- 
Leah Neukirchen  <leah@vuxu.org>  https://leahneukirchen.org/

Re: runit: tryshsgr.c: why not test whether gid_t is short by using "sizeof(short) == sizeof(gid_t)"

[Zhixu Liu]

Get and confirm it, thanks for the reply.

On Wed, Sep 4, 2024 at 7:56 PM Leah Neukirchen <leah@vuxu.org> wrote:
>
> Zhixu Liu <zhixu.liu@gmail.com> writes:
>
> > Hi,
> >
> > I'm trying to keep runit remaining available in gentoo's official portage tree,
> > see https://bugs.gentoo.org/938282 . I setup a github repo at
> > https://github.com/clan/runit/ for this purpose, following is what I'm doing
> > now:
> > 1. fix all the compilation error(s)
> > 2. eliminate the compilation warning(s) as much as possible
> >
> > Now I 'm in stage 2, when fixing the warning of
> > -Wincompatible-pointer-types, see
> > https://github.com/clan/runit/blob/master/admin/runit-2.1.2/src/tryshsgr.c
> >
> > As G. Pape has commented at
> > https://github.com/clan/runit/commit/5a7b8099ae638ce72abae2c4d8cac7ce33477522
> >
> >> "tryshsgr" is specifically there to check whether the size of gid_t is short,
> >> it doesn't matter if it fails at compile time or run time. But with this
> >> change, it always succeeds, and so always size short is
> >> assumed, even though most systems should have unsigned int I guess.
>
> As of runit.git 84457fd2 (2005-08-23), prot_gid is unused and all of
> this is dead code that can be removed.  setgroups is used directly
> with proper gid_t types.
>
> --
> Leah Neukirchen  <leah@vuxu.org>  https://leahneukirchen.org/



-- 
Z. Liu