Re: [PATCH] libacl: use getpwnam_r and getgrnam_r in acl_from_text.c

[Mike Frysinger]

On 20 Jul 2022 15:33, lzaoral@redhat.com wrote:
> +static int
> +allocate_buffer(char **buffer, long *bufsize, int type)
> +{
> +     *bufsize = sysconf(type);
> +     if (*bufsize == -1)

prob should be <=0 for safety sake.  getpwnam_r treats the length as
(unsigned) size_t.

> +             *bufsize = 16384;

that's not really guaranteed to be big enough.  for that matter, the
return value of sysconf isn't guaranteed either.  i know the type is
called "size max", but it's actually the "suggested initial size".

i guess the question is, do we keep handling ERANGE errors (by trying
to increase the buffer size), or not bother.  if we don't want to bother,
leave a comment explaining that we're being lazy.

> +     *buffer = malloc(*bufsize);
> +     if (*buffer == NULL)
> +             return -1;
> +
> +     return 0;

this func looks like it was written in C++.  do we really need to have
dedicated output params ?  the code guarantees that:
        (ret=0 && buffer!=NULL) || (ret!=0 && buffer==NULL)
so just return the pointer directly ?

>  static int
>  get_uid(const char *token, uid_t *uid_p)
>  {
> ...
> +     char * buffer;

no space after the *

>  static int
>  get_gid(const char *token, gid_t *gid_p)
>  {
> ...
> +     char * buffer;

here too
-mike

signature.asc
Description: PGP signature