Re: [PATCH] lib/rename.c: rpl_rename - Avoid unused-but-set-variable compiler warning
On 06/02/2014 12:45 PM, Ben Walton wrote: The value is only referenced if RENAME_DEST_EXISTS_BUG is set. Otherwise, it's set but never used again. This patch addresses the case where RENAME_DEST_EXISTS_BUG is unset. Thanks -Ben Since that's not obvious, it might be a bit cleaner to have something like this instead: # if RENAME_DEST_EXISTS_BUG bool dst_exists = false; # endif ... # if RENAME_DEST_EXISTS_BUG dst_exists = true; # endif
Re: [PATCH] lib/rename.c: rpl_rename - Avoid unused-but-set-variable compiler warning
The value is set in se On Mon, Jun 2, 2014 at 8:31 PM, Pádraig Brady wrote: > On 06/02/2014 08:13 PM, Ben Walton wrote: >> * In the non-Win32 variant of rpl_rename, it is possible that >> dst_exists may be set but not used. Mark it with the unused >> attribute to avoid compiler warnings. >> >> Signed-off-by: Ben Walton >> --- >> lib/rename.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/rename.c b/lib/rename.c >> index 55130d8..099066d 100644 >> --- a/lib/rename.c >> +++ b/lib/rename.c >> @@ -285,7 +285,7 @@ rpl_rename (char const *src, char const *dst) >>char *dst_temp = (char *) dst; >>bool src_slash; >>bool dst_slash; >> - bool dst_exists; >> + bool dst_exists _GL_UNUSED; >>int ret_val = -1; >>int rename_errno = ENOTDIR; >>struct stat src_st; >> > > Have to say that I don't see this from looking at the code, > since it seems to just return or set dst_exists? The value is only referenced if RENAME_DEST_EXISTS_BUG is set. Otherwise, it's set but never used again. This patch addresses the case where RENAME_DEST_EXISTS_BUG is unset. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens ---
Re: [PATCH] lib/rename.c: rpl_rename - Avoid unused-but-set-variable compiler warning
On 06/02/2014 08:13 PM, Ben Walton wrote: > * In the non-Win32 variant of rpl_rename, it is possible that > dst_exists may be set but not used. Mark it with the unused > attribute to avoid compiler warnings. > > Signed-off-by: Ben Walton > --- > lib/rename.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/rename.c b/lib/rename.c > index 55130d8..099066d 100644 > --- a/lib/rename.c > +++ b/lib/rename.c > @@ -285,7 +285,7 @@ rpl_rename (char const *src, char const *dst) >char *dst_temp = (char *) dst; >bool src_slash; >bool dst_slash; > - bool dst_exists; > + bool dst_exists _GL_UNUSED; >int ret_val = -1; >int rename_errno = ENOTDIR; >struct stat src_st; > Have to say that I don't see this from looking at the code, since it seems to just return or set dst_exists? Pádraig.
Re: [PATCH] lib/rename.c: Conditionally define the out label
On Mon, Jun 2, 2014 at 3:02 PM, Paul Eggert wrote: > Ben Walton wrote: >> >> With a new _GL_UNUSED_LABEL, which is cleaner than my patch, I'm less >> inclined. > > > Then please go with that. It's not a big deal either way. Done. Updated patch sent. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens ---
Re: [PATCH] lib/rename.c: rpl_rename - mark the out label as potentially unused
On 06/02/2014 07:50 PM, Ben Walton wrote: > * Avoid possible compiler warnings/errors by marking the out label > as potentially unused. > > Signed-off-by: Ben Walton > --- > lib/rename.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/rename.c b/lib/rename.c > index 2116028..55130d8 100644 > --- a/lib/rename.c > +++ b/lib/rename.c > @@ -462,7 +462,9 @@ rpl_rename (char const *src, char const *dst) > >ret_val = rename (src_temp, dst_temp); >rename_errno = errno; > - out: > + > + out: _GL_UNUSED_LABEL > + >if (src_temp != src) > free (src_temp); >if (dst_temp != dst) I pushed that with a trailing ; needed for C++, along with a comment stating the need for the trailing ; thanks, Pádraig.
[PATCH] lib/rename.c: rpl_rename - Avoid unused-but-set-variable compiler warning
* In the non-Win32 variant of rpl_rename, it is possible that dst_exists may be set but not used. Mark it with the unused attribute to avoid compiler warnings. Signed-off-by: Ben Walton --- lib/rename.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rename.c b/lib/rename.c index 55130d8..099066d 100644 --- a/lib/rename.c +++ b/lib/rename.c @@ -285,7 +285,7 @@ rpl_rename (char const *src, char const *dst) char *dst_temp = (char *) dst; bool src_slash; bool dst_slash; - bool dst_exists; + bool dst_exists _GL_UNUSED; int ret_val = -1; int rename_errno = ENOTDIR; struct stat src_st; -- 1.9.1
[PATCH] lib/rename.c: rpl_rename - mark the out label as potentially unused
* Avoid possible compiler warnings/errors by marking the out label as potentially unused. Signed-off-by: Ben Walton --- lib/rename.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rename.c b/lib/rename.c index 2116028..55130d8 100644 --- a/lib/rename.c +++ b/lib/rename.c @@ -462,7 +462,9 @@ rpl_rename (char const *src, char const *dst) ret_val = rename (src_temp, dst_temp); rename_errno = errno; - out: + + out: _GL_UNUSED_LABEL + if (src_temp != src) free (src_temp); if (dst_temp != dst) -- 1.9.1
Re: getpt support?
On 06/02/2014 11:12 AM, Gavin Smith wrote: (i) posix_openpt isn't documented in the glibc manual (but I've raised a bug for this), and (ii) posix_openpt has an ugly name. I wouldn't worry about the lack of documentation; posix_openpt is not going away any time soon. Admittedly the name is ugly, but portability should trump that minor wart.
Re: getpt support?
On Mon, Jun 2, 2014 at 4:47 PM, Paul Eggert wrote: > Why not just use posix_openpt? If the two functions are exactly the same (I think they are), this would be acceptable. There are two weak reasons to use getpt instead: (i) posix_openpt isn't documented in the glibc manual (but I've raised a bug for this), and (ii) posix_openpt has an ugly name. There is probably not a strong need for it but I would use it if it were there.
Re: getpt support?
Why not just use posix_openpt?
getpt support?
I was wondering what the chances would be of getting support for the getpt function in gnulib? It's a GNU stdlib.h extension to get a file descriptor for a master pseudo-terminal. As far as I can tell it does something very similar to posix_openpt, and there is already a gnulib posix_openpt module. Thanks, Gavin
Re: hasmntopt compiler warning
Ben Walton wrote: Should I simply add a conditional logic to detect Solaris (and later other platforms) that exhibit this mismatch That'd be OK, thanks. If someone wants to do something fancier that'd be even better.
Re: [PATCH] lib/rename.c: Conditionally define the out label
Ben Walton wrote: With a new _GL_UNUSED_LABEL, which is cleaner than my patch, I'm less inclined. Then please go with that. It's not a big deal either way.
Re: Solaris acl woes
Ben Walton wrote: The lib/file-has-acl.c:acl_ace_nontrivial code that returns 1 is: Why is it returning 1, exactly? What are the value of access_masks[0, 1] and how do they compare to the masks, and what bits are set that shouldn't be if we want the ACLs to be trivial?
Re: autoconf warning in gl_EARLY
On 06/01/2014 12:16 PM, Denis Laroche wrote:
> I'm not sure if the post should be sent to the autoconf mailing list
> instead. I see the following warnings from gl_EARLY when running
> autoconf (version 2.69):
The bug is on your end, not in gnulib.
> The version of gnulib is 0.1.147-b1b4ba. Looking at the definition of
> gl_EARLY, it's expanding macro gl_USE_SYSTEM_EXTENSIONS. The
> documenation of this macro says that its purpose is to avoid those
> warnings. The rest of the message shows the beginning of configure.ac.
And for gl_EARLY to do its job, it MUST be called early.
>
> Thanks in advance!
>
> AC_PREREQ([2.69])
>
> AC_INIT([lvibs], [0.1.0], [lvibs...@gmail.ckom])
> AC_CONFIG_AUX_DIR([build-aux])
> AC_CONFIG_MACRO_DIR([m4])
> AC_CONFIG_SRCDIR([src/lvibs.c])
>
> AM_INIT_AUTOMAKE([1.11 color-tests subdir-objects -Wall])
> LT_PREREQ([2.2])
call it here...
> AM_PROG_AR
> LT_INIT
>
> AM_MAINTAINER_MODE([enable])
>
> # Checks for programs.
> AC_PROG_CC
> AC_PROG_INSTALL
> AM_PROG_CC_C_O
>
> # Checks for optional programs.
> PROG_TRY_DOXYGEN
>
> AM_CONDITIONAL([HAVE_DOXYGEN], [test -n "$DOXYGEN"])
> AS_IF([test -n "$DOXYGEN"], [AC_CONFIG_FILES([apidoc/doxyfile])])
>
> AC_LANG([C])
>
> AC_PROG_CC_C99
> if test "$x{ac_cv_prog_cc_c99}" = xno
> then
> AC_MSG_FAILURE([
>
> A C99 compiler is required to build the
> program.
>
> ])
> fi
>
> gl_EARLY
...not here.
> gl_INIT
>
>
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: Memory Leaks
On 06/01/2014 12:39 AM, Geoff wrote: > I am (would be) _very_ happy - ecstatic even - about my present situation. My > concern is one regarding certain memory leaks I found I could not avoid with > regard to parent processes. I had hoped to limit the page tables accessed by > these parent processes; although it appears I may not have fully done the > best I could with regard to this. But by personal opinion is that it should > not pose a critical security issue. > > Instead, I seem to be reading in the documentation much more to the effect of > “bugs are normal”. This I get. I can work out my bugs. What I’m unclear about > is whether these leaks are/were sufficient such that management feels this > program is no longer viable...? Your mail is missing some context. I have no idea _which_ program you are complaining about, or if there is a pointer to an earlier email thread that has more background on the subject. Are you reporting a bug against gnulib? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Solaris acl woes
Hi All,
I've hit a "bug" in the coreutils test suite when exercising acl code
from gnulib. I'm cross-posting to both lists, I hope that's ok.
(Please apply the cluebat gently if not...)
When running tests involving cp -a (in /tmp for this example), I see
errors like:
+ cp -a --parents d/a/b/c e
cp: preserving permissions for 'e/d/a/b/c': Operation not applicable
cp: preserving permissions for 'e/d/a/b': Operation not applicable
+ fail=1
+ cp -a --parents sym/b/c g
cp: preserving permissions for 'g/sym/b/c': Operation not applicable
cp: preserving permissions for 'g/sym/b': Operation not applicable
The code that results in this error in in lib/qcopy-acl.c:
# ifdef ACE_GETACL
if (ace_count > 0)
{
ret = (dest_desc != -1
? facl (dest_desc, ACE_SETACL, ace_count, ace_entries)
: acl (dst_name, ACE_SETACL, ace_count, ace_entries));
if (ret < 0 && saved_errno == 0)
{
saved_errno = errno;
if ((errno == ENOSYS || errno == EINVAL || errno == ENOTSUP)
&& !acl_ace_nontrivial (ace_count, ace_entries))
saved_errno = 0;
}
}
free (ace_entries);
# endif
The error returned is ENOSYS, but acl_ace_nontrivial fires and thus
the saved_errno isn't reset to 0.
The lib/file-has-acl.c:acl_ace_nontrivial code that returns 1 is:
switch ((access_masks[0] | access_masks[1])
& ~(NEW_ACE_READ_NAMED_ATTRS
| NEW_ACE_READ_ATTRIBUTES
| NEW_ACE_READ_ACL
| NEW_ACE_SYNCHRONIZE))
{
case 0:
case NEW_ACE_READ_DATA:
case NEW_ACE_WRITEA_DATA:
case NEW_ACE_READ_DATA | NEW_ACE_WRITEA_DATA:
case NEW_ACE_EXECUTE:
case NEW_ACE_READ_DATA | NEW_ACE_EXECUTE:
case NEW_ACE_WRITEA_DATA | NEW_ACE_EXECUTE:
case NEW_ACE_READ_DATA | NEW_ACE_WRITEA_DATA | NEW_ACE_EXECUTE:
break;
default:
return 1;
}
It seems as though the $getacl calls succeed but $setacl calls fail.
We are able to retrieve what we consider complex acl information but
cannot subsequently set it.
You can see similar behaviour by doing:
$ pwd
/tmp/coreutils-8.22
$ getfacl README
# file: README
# owner: bwalton
# group: csw
user::rw-
group::r-- #effective:r--
mask:rwx
other:r--
$ getfacl README | setfacl -f - README
README: failed to set acl entries
setacl error: Operation not applicable
Other than this issue, which affects 6 tests identically, all tests pass.
I'm not sure what the best way to fix this is. I'm happy to supply
complete test logs if they'd help and will run any commands/gather any
additional info you need.
(Note: I'm only running this in /tmp to isolate some other issues that
seem to be nfs related - I'll report on those separately when I've
debugged far enough to provide something useful.)
Thanks
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
hasmntopt compiler warning
Hi All, When building coreutils on solaris 10 x86 with warnings as errors, the mountlist.c code using hasmntopt complains about passing a const char to a function expecting char. Solaris defines hasmntopt as: char *hasmntopt(struct mnttab *, char *); While it's easy to patch around locally, by doing: -# define MNT_IGNORE(M) hasmntopt (M, MNTOPT_IGNORE) +# define MNT_IGNORE(M) hasmntopt (M, (char *) MNTOPT_IGNORE) This isn't something that is valid to send upstream. I'm not sure what the cleanest way to handle this is. Should I simply add a conditional logic to detect Solaris (and later other platforms) that exhibit this mismatch or is there a better mechanism I should use? Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens ---
Re: [PATCH] lib/rename.c: Conditionally define the out label
On Sun, Jun 1, 2014 at 11:47 PM, Paul Eggert wrote: > Ben Walton wrote: >> >> WIth your suggested followup >> (prerequisite), I think that it's a better solution. > > > Better yet, how about putting the goto-containing code into a subsdiary > static function that can return rather than goto? That'd be cleaner anyway. I had considered this, but it didn't feel like the right choice to me. With a new _GL_UNUSED_LABEL, which is cleaner than my patch, I'm less inclined. That said, if you folks feel it's the way to go, I will write the patch to meet your ideals which are more important. :) Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens ---
