Re: [LTP] [PATCH v3 1/1] setgroups03: Fix running more iterations (-i 2)

zhaogongyi
2021-10-14T02:44:52Z

Hi,

Yes, in this testcase, the groups_list is redundant and can be removed.

I am sorry for my late reply.

Thanks so much!

> > > -int setup1(void)
> > > +void setup1(const char *uid, uid_t euid)
> > > {
> > > - struct passwd *user_info; /* struct. to hold test user info */
> > > -
> > > -/* Switch to nobody user for correct error code collection */
> > > - ltpuser = getpwnam(nobody_uid);
> > > - if (seteuid(ltpuser->pw_uid) == -1) {
> > > - tst_resm(TINFO, "setreuid failed to "
> > > - "to set the effective uid to %d", ltpuser->pw_uid);
> > > - perror("setreuid");
> > > - }
> > > + struct passwd *user_info;
>
> > > - if ((user_info = getpwnam(TESTUSER)) == NULL) {
> > > - tst_brkm(TFAIL, cleanup, "getpwnam(2) of %s Failed",
> TESTUSER);
> > > - }
> > > + SAFE_SETEUID(cleanup, euid);
> > > +
> > > + user_info = SAFE_GETPWNAM(cleanup, uid);
>
> > I still do not get why we call SAFE_GETPWNAM() here. We should do that
> > in the setup and prepare two different group_list[] lists, if that is
> > really needed.
>
> > But I guess that all we need in this test is:
>
> > * Run the EINVAL test as a root
>
> > * Run the EPERM test as a nobody
>
> > The content of the list should not matter, as a matter of a fact we
> > pass unitialized data in the EINVAL case. What matters is the size
> > argument, it should be 1 for the EPERM test and max+1 for the EINVAL
> case.
>
> Good point, thank you!
>
> @Zhao feel free to let me know you're doing to implement it.
> Otherwise I'll have look on Monday.
>
> Kind regards,
> Petr
>
> > > if (!GID_SIZE_CHECK(user_info->pw_gid)) {
> > > tst_brkm(TBROK,
> > > cleanup,
> > > "gid returned from getpwnam is too large for testing
> setgroups16");
> > > }
> > > +
> > > groups_list[0] = user_info->pw_gid;
> > > - return 0;
> > > }
>
> > > /*
> > > --
> > > 2.33.0