[gccsdk] Patch for getgrouplist() and ELF questions

John Tytgat John.Tytgat at aaug.net
Sat Dec 6 06:20:45 PST 2008


In message <20081204215433.GA19708 at chiark.greenend.org.uk>
          Theo Markettos <theo at markettos.org.uk> wrote:

> Anyway, attached is a patch to provide getgrouplist() for review.  This
> enables coreutils to build.  I haven't been able to test it on RISC OS, but
> there's a test program and file included, which covers it fairly thoroughly. 

The availability of a test program is appreciated.

> As testing requires a known /etc/group (where does that end up on RISC OS?)

I guess UnixLib will search for $.etc.group on whatever your current FS is.
Perhaps this feature calls for a small change in !UnixHome in our
Autobuilder.  UnixLib is able to map (Unix) directories located under
the root to specific RISC OS directories as one of the many things it is
doing when converting Unix filespecs to RISC OS filespecs.

Cfr. the UnixFS$/* system variables description in UnixLib README.  So

*Set UnixFS$/etc <UnixHome$Dir>.etc

in !UnixHome and create an etc subdirectory holding an appropriate group
file ?

> the test program needed a modified copy of the function to work on Unix.  So
> it's only a regression test if run on RISC OS and you can introduce the
> supplied 'test.group' file as /etc/group for the duration of the test.

> Index: gcc4/recipe/files/libunixlib/grp/getgroups.c
> ===================================================================
> --- gcc4/recipe/files/libunixlib/grp/getgroups.c	(revision 3564)
> +++ gcc4/recipe/files/libunixlib/grp/getgroups.c	(working copy)
> @@ -25,6 +25,102 @@

I would break out this getgrouplist() implementation into a new file in
grp directory.  When you would do that, add something like this:

/* getgrouplist ()
 * Copyright (c) 2008 UnixLib Developers
 * Written by Theo Markettos.
 */

+ a set of include headers at the top.

This would avoid having two nearly same versions here and in the test
directory as well (see further on).

> [...]
> +Return Value
> +
> +On success, if there was sufficient room to copy all the supplementatry
> +group identifiers to the array identified by groups, getgrouplist() shall
> +return the number of gid_t objects copied, and the value referenced by
> +ngroups shall be updated. If there was not sufficient room to copy all the
> +supplementary group identifiers, grouplist() shall return -1, and update the
                                    ^^^^^^^^^

getgrouplist ?

> [...]
> +  /* Iterate through the group structure, copying gids of groups
> +   * we find that match.  Leave at least one space at the end.
> +   */
> +  while ((grp = getgrent ()) !=NULL)
> +    {
> +      char **mem;

const char ** would be slightly better.

> [...]
> Index: gcc4/recipe/files/libunixlib/test/grp/getgrouplist.c
> ===================================================================
> --- gcc4/recipe/files/libunixlib/test/grp/getgrouplist.c	(revision 0)
> +++ gcc4/recipe/files/libunixlib/test/grp/getgrouplist.c	(revision 0)
> @@ -0,0 +1,214 @@
> +/*
> + * Test function for getgrouplist()
> + *
> + * If built for RISC OS, will use the group file in /etc/group:
> + * need to copy the file test.groups there before running the test
> + * (and probably replace it afterwards with what was there before)
> + *
> + * If built for other platforms it'll use the host C library functions to
> + * read the file 'test.group' in the current directory.  It'll only test
> + * the copy of getgrouplist() found in this file, not the one in
> + * the main UnixLib tree.
> + *
> + * Copyright (c) 2002-2008 UnixLib Developers

Probably 2008 is enough ;-)

> + */
> +
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <grp.h>
> +#include <pthread.h>
> +
> +#ifndef __riscos
> +
> +
> +/* getgrouplist():
> [...]

What about doing a #include "../../grp/getgrouplist.c" instead (assuming you
have your getgrouplist implementation in a separate file) and have the
differences between this version and the UnixLib implementation covered with
a test on __riscos ? So that we're sure that any changes in
libunixlib/grp/getgrouplist.c are under test when compiling and running
libunixlib/test/grp/getgrouplist.c ?

No comments on the other parts.  Thanks.

John.
-- 
John Tytgat, in his comfy chair at home                                 BASS
John.Tytgat at aaug.net                             ARM powered, RISC OS driven




More information about the gcc mailing list