Re: [PATCH] rrd_daemon: pay attention to the return value of realpath

It looks like setting the 2nd arg of realpath() to NULL is not a portable
way to cause it to be strdup()'ed.  i.e. on Solaris 2.8:

     EINVAL
           Either the file_name or resolved_name  argument  is  a
           null pointer.

So perhaps keeping the original strdup(realpath()) would make more sense.

Also, checking specifically for error of realpath() and returning on error
might shrink the diff.  Style change only.

-kb

On Sat, Nov 13, 2010 at 4:47 AM, Alex Bennee <[email protected]> wrote:

> When the realpath() change was introduced it failed to take account of the
> potential
> for a failure of realpath if it can't navigate to the full path. As a
> result the strdup
> would fail.
>
> Unfortunatly this change broke rrdcached's automatic creation of it's
> journal path (although
> in my case this is handled by packaging). However we still make the call to
> rrd_mkdir_p to
> trap other cases like existing non-dirs. It also removes the strdup as
> realpath will allocate
> a buffer for you if you ask.
> ---
>  src/rrd_daemon.c |   40 ++++++++++++++++++++++------------------
>  1 files changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
> index 2209320..5e3f89e 100644
> --- a/src/rrd_daemon.c
> +++ b/src/rrd_daemon.c
> @@ -3278,24 +3278,28 @@ static int read_options (int argc, char **argv) /*
> {{{ */
>
>       case 'j':
>       {
> -        char journal_dir_actual[PATH_MAX];
> -        const char *dir;
> -        dir = journal_dir = strdup(realpath((const char *)optarg,
> journal_dir_actual));
> -
> -        status = rrd_mkdir_p(dir, 0777);
> -        if (status != 0)
> -        {
> -          fprintf(stderr, "Failed to create journal directory '%s': %s\n",
> -              dir, rrd_strerror(errno));
> -          return 6;
> -        }
> -
> -        if (access(dir, R_OK|W_OK|X_OK) != 0)
> -        {
> -          fprintf(stderr, "Must specify a writable directory with -j!
> (%s)\n",
> -                  errno ? rrd_strerror(errno) : "");
> -          return 6;
> -        }
> +       journal_dir = realpath((const char *)optarg, NULL);
> +       if (journal_dir)
> +       {
> +         // a resolved realpath implies existing path, however rrd_mkdir_p
> also runs checks
> +         status = rrd_mkdir_p(journal_dir, 0777);
> +         if (status != 0)
> +         {
> +           fprintf(stderr, "Failed to create journal directory '%s':
> %s\n",
> +                   journal_dir, rrd_strerror(errno));
> +           return 6;
> +         }
> +         if (access(journal_dir, R_OK|W_OK|X_OK) != 0)
> +         {
> +           fprintf(stderr, "Must specify a writable directory with -j!
> (%s)\n",
> +                   errno ? rrd_strerror(errno) : "");
> +           return 6;
> +         }
> +       } else {
> +         fprintf(stderr, "Unable to resolve journal path (%s,%s)\n",
> optarg,
> +                 errno ? rrd_strerror(errno) : "");
> +         return 6;
> +       }
>       }
>       break;
>
> --
> 1.7.3.2
>
> _______________________________________________
> rrd-developers mailing list
> [email protected]
> https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
>



-- 
 kevin brintnall =~ /[email protected]/

_______________________________________________
rrd-developers mailing list
[email protected]
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers