xfs
[Top] [All Lists]

Re: [PATCH] Validate string -> number conversion.

To: Arkadiusz Miśkiewicz <arekm@xxxxxxxx>
Subject: Re: [PATCH] Validate string -> number conversion.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 26 Aug 2010 18:26:12 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1282807852-23457-1-git-send-email-arekm@xxxxxxxx>
References: <1282724556-19456-1-git-send-email-arekm@xxxxxxxx> <1282807852-23457-1-git-send-email-arekm@xxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Thu, Aug 26, 2010 at 09:30:52AM +0200, Arkadiusz Miśkiewicz wrote:
> Make sure that numbers passed as string will fit into proper
> types when doing string->uid_t/gid_t/prid_t conversion.
> 
> Signed-off-by: Arkadiusz Miśkiewicz <arekm@xxxxxxxx>

Looks good, but just a couple of minor style things:

> ---
> 
> Fixed version.
> 
>  libxcmd/input.c |   33 +++++++++++++++++++++------------
>  quota/project.c |    2 +-
>  2 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/libxcmd/input.c b/libxcmd/input.c
> index 1bc0745..d0c4e0d 100644
> --- a/libxcmd/input.c
> +++ b/libxcmd/input.c
> @@ -336,16 +336,19 @@ prid_from_string(
>       char            *project)
>  {
>       fs_project_t    *prj;
> -     prid_t          prid;
> +     unsigned long int       prid_long;
>       char            *sp;

unsigned long is all that is necessary here - that will also keep
the same indent as well.
>  
>       /*
>        * Allow either a full numeric or a valid projectname, even
>        * if it starts with a digit.
>        */
> -     prid = (prid_t)strtoul(project, &sp, 10);
> -     if (*project != '\0' && *sp == '\0')
> -             return prid;
> +     prid_long = strtoul(project, &sp, 10);
> +     if (*project != '\0' && *sp == '\0') {
> +             if ((prid_long == ULONG_MAX && errno == ERANGE) || (prid_long > 
> (prid_t)-1))

Can you split this to keep within 80 chars?

Same for the uid and gid checks as well.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>