[prev in list] [next in list] [prev in thread] [next in thread]
List: rrd-developers
Subject: Re: [rrd-developers] [PATCH] rrd_daemon: pay attention to the
From: kevin brintnall <kbrint () rufus ! net>
Date: 2010-11-22 17:09:26
Message-ID: AANLkTin05b9XvhZ1fnoqEoPmQ7MZefUwrSF8aazQHMaN () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Alex, realpath(x,NULL) is not portable. I think keeping the temporary
buffer is best. Ideas?
-kb
On Mon, Nov 22, 2010 at 9:17 AM, <kernel-hacker@bennee.com> wrote:
> From: Alex Bennee <ajb@cbnl.com>
>
> 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
> rrd-developers@lists.oetiker.ch
> https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
>
--
kevin brintnall =~ /kbrint@rufus.net/
[Attachment #5 (text/html)]
Alex, realpath(x,NULL) is not portable. I think keeping the temporary buffer is \
best. Ideas?<div><br></div><div>-kb<br><br><div class="gmail_quote">On Mon, Nov 22, \
2010 at 9:17 AM, <span dir="ltr"><<a \
href="mailto:kernel-hacker@bennee.com">kernel-hacker@bennee.com</a>></span> \
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex;">From: Alex Bennee <<a \
href="mailto:ajb@cbnl.com">ajb@cbnl.com</a>><br> <br>
When the realpath() change was introduced it failed to take account of the \
potential<br> for a failure of realpath if it can't navigate to the full path. As \
a result the strdup<br> would fail.<br>
<br>
Unfortunatly this change broke rrdcached's automatic creation of it's journal \
path (although<br> in my case this is handled by packaging). However we still make \
the call to rrd_mkdir_p to<br> trap other cases like existing non-dirs. It also \
removes the strdup as realpath will allocate<br> a buffer for you if you ask.<br>
---<br>
src/rrd_daemon.c | 40 ++++++++++++++++++++++------------------<br>
1 files changed, 22 insertions(+), 18 deletions(-)<br>
<br>
diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c<br>
index 2209320..5e3f89e 100644<br>
--- a/src/rrd_daemon.c<br>
+++ b/src/rrd_daemon.c<br>
@@ -3278,24 +3278,28 @@ static int read_options (int argc, char **argv) /* {{{ */<br>
<br>
case 'j':<br>
{<br>
- char journal_dir_actual[PATH_MAX];<br>
- const char *dir;<br>
- dir = journal_dir = strdup(realpath((const char *)optarg, \
journal_dir_actual));<br>
-<br>
- status = rrd_mkdir_p(dir, 0777);<br>
- if (status != 0)<br>
- {<br>
- fprintf(stderr, "Failed to create journal directory '%s': \
%s\n",<br>
- dir, rrd_strerror(errno));<br>
- return 6;<br>
- }<br>
-<br>
- if (access(dir, R_OK|W_OK|X_OK) != 0)<br>
- {<br>
- fprintf(stderr, "Must specify a writable directory with -j! \
(%s)\n",<br>
- errno ? rrd_strerror(errno) : "");<br>
- return 6;<br>
- }<br>
+ journal_dir = realpath((const char *)optarg, NULL);<br>
+ if (journal_dir)<br>
+ {<br>
+ // a resolved realpath implies existing path, however rrd_mkdir_p also runs \
checks<br> + status = rrd_mkdir_p(journal_dir, 0777);<br>
+ if (status != 0)<br>
+ {<br>
+ fprintf(stderr, "Failed to create journal directory '%s': \
%s\n",<br> + journal_dir, rrd_strerror(errno));<br>
+ return 6;<br>
+ }<br>
+ if (access(journal_dir, R_OK|W_OK|X_OK) != 0)<br>
+ {<br>
+ fprintf(stderr, "Must specify a writable directory with -j! \
(%s)\n",<br> + errno ? rrd_strerror(errno) : \
"");<br> + return 6;<br>
+ }<br>
+ } else {<br>
+ fprintf(stderr, "Unable to resolve journal path (%s,%s)\n", \
optarg,<br> + errno ? rrd_strerror(errno) : "");<br>
+ return 6;<br>
+ }<br>
}<br>
break;<br>
<font color="#888888"><br>
--<br>
1.7.3.2<br>
</font><div><div></div><div class="h5"><br>
_______________________________________________<br>
rrd-developers mailing list<br>
<a href="mailto:rrd-developers@lists.oetiker.ch">rrd-developers@lists.oetiker.ch</a><br>
<a href="https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers" \
target="_blank">https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers</a><br> \
</div></div></blockquote></div><br><br clear="all"><br>-- <br> kevin brintnall =~ /<a \
href="http://kbrint@rufus.net/">kbrint@rufus.net/</a><br><br> </div>
_______________________________________________
rrd-developers mailing list
rrd-developers@lists.oetiker.ch
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic