Memory Leaks
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...?
[PATCH] acl: Address pure attribute errors with gcc 4.9
* lib/acl-internal.h (acl_ace_nontrivial): Apply pure attribute * lib/file-has-acl.c: Disable pure attribute error. - The AIX version of acl_nontrivial isn't pure while other versions are. We cannot apply the attribute globally. Signed-off-by: Ben Walton --- When building coreutils 8.22 on Solaris 10 x86, gcc 4.9.0 emits: ..snip.. b/file-has-acl.c: In function 'acl_nontrivial': lib/file-has-acl.c:133:1: error: function might be candidate for attribute 'pure' [-Werror=suggest-attribute=pure] acl_nontrivial (int count, aclent_t *entries) ^ lib/file-has-acl.c: In function 'acl_ace_nontrivial': lib/file-has-acl.c:164:1: error: function might be candidate for attribute 'pure' [-Werror=suggest-attribute=pure] acl_ace_nontrivial (int count, ace_t *entries) ^ ..snip.. It seems that acl_ace_nontrivial is pure, so mark it as such. For acl_nontrivial though, the AIX version isn't pure, so disable the warning instead. I'm not keen on squashing the warning for all versions of the function, but applying the attribute selectively doesn't feel right either. Maybe I'm missing the nicest solution entirely? lib/acl-internal.h | 2 +- lib/file-has-acl.c | 8 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/acl-internal.h b/lib/acl-internal.h index fdffe64..8c26dae 100644 --- a/lib/acl-internal.h +++ b/lib/acl-internal.h @@ -181,7 +181,7 @@ extern int acl_nontrivial (int count, aclent_t *entries); /* Test an ACL retrieved with ACE_GETACL. Return 1 if the given ACL, consisting of COUNT entries, is non-trivial. Return 0 if it is trivial, i.e. equivalent to a simple stat() mode. */ -extern int acl_ace_nontrivial (int count, ace_t *entries); +extern int acl_ace_nontrivial (int count, ace_t *entries) _GL_ATTRIBUTE_PURE; /* Definitions for when the built executable is executed on Solaris 10 (newer version) or Solaris 11. */ diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c index 5104a41..3ac12e3 100644 --- a/lib/file-has-acl.c +++ b/lib/file-has-acl.c @@ -23,6 +23,14 @@ # pragma GCC diagnostic ignored "-Wsuggest-attribute=const" #endif +/* Without this pragma, gcc 4.9.0 may suggest that the + acl_nontrivial function might be a candidate for attribute + 'pure'. The AIX version of the function invalidates the + suggestion. The other versions would be ok.*/ +#if (__GNUC__ == 4 && 9 <= __GNUC_MINOR__) || 4 < __GNUC__ +# pragma GCC diagnostic ignored "-Wsuggest-attribute=pure" +#endif + #include #include "acl.h" -- 1.9.1
parse-duration.c - TIME_MAX 2038 unpreparedness
Hi, I noticed that lib/parse-duration.c does: #define TIME_MAX 0x7FFF This is naturally entirely unacceptable as the 2038 bug ((time_t) INT32_MAX) is coming up soon and most operating systems are 64-bit time_t capable by now. This should be rewritten as something involving sizeof(time_t) in a safe and portable manner. It would probably be best to not use a define named TIME_MAX, as that name would be ideal if a standard header wanted to expose the domain of time_t for use in cases like this (indeed, this came up because I am adding a TIME_MAX constant to my libc). Jonas 'Sortie' Termansen
[PATCH] lib/rename.c: Conditionally define the out label
* Avoid possible compiler warnings/errors by defining the out label only when it may be accessed. Signed-off-by: Ben Walton --- Hi All, When building coreutils 8.22 on Solaris with -Werror=unused-label, the build fails with: lib/rename.c: In function 'rpl_rename': lib/rename.c:465:2: error: label 'out' defined but not used [-Werror=unused-label] out: ^ I think this should make the compiler happier. Feel free to suggest better solutions though. I'm not sure this is the best way to handle it. lib/rename.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/lib/rename.c b/lib/rename.c index 2116028..9c507c2 100644 --- a/lib/rename.c +++ b/lib/rename.c @@ -462,7 +462,14 @@ rpl_rename (char const *src, char const *dst) ret_val = rename (src_temp, dst_temp); rename_errno = errno; + +# if (RENAME_TRAILING_SLASH_SOURCE_BUG || RENAME_DEST_EXISTS_BUG\ + || RENAME_HARD_LINK_BUG) + /* Avoid compiler warnings about unused labels. Only + create this label if it will be used. */ out: +# endif + if (src_temp != src) free (src_temp); if (dst_temp != dst) -- 1.9.1
autoconf warning in gl_EARLY
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):
configure.ac:42: warning: AC_RUN_IFELSE was called before
AC_USE_SYSTEM_EXTENSIONS
../../lib/autoconf/specific.m4:314: AC_GNU_SOURCE is expanded from...
m4/gnulib-comp.m4:34: gl_EARLY is expanded from...
configure.ac:42: the top level
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.
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])
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
gl_INIT
Re: [PATCH] acl: Address pure attribute errors with gcc 4.9
On 06/01/2014 09:42 AM, Ben Walton wrote: > * lib/acl-internal.h (acl_ace_nontrivial): Apply pure attribute > * lib/file-has-acl.c: Disable pure attribute error. > - The AIX version of acl_nontrivial isn't pure while other > versions are. We cannot apply the attribute globally. > > Signed-off-by: Ben Walton > --- > When building coreutils 8.22 on Solaris 10 x86, gcc 4.9.0 emits: > > ..snip.. > b/file-has-acl.c: In function 'acl_nontrivial': > lib/file-has-acl.c:133:1: error: function might be candidate for attribute > 'pure' [-Werror=suggest-attribute=pure] > acl_nontrivial (int count, aclent_t *entries) > ^ > lib/file-has-acl.c: In function 'acl_ace_nontrivial': > lib/file-has-acl.c:164:1: error: function might be candidate for attribute > 'pure' [-Werror=suggest-attribute=pure] > acl_ace_nontrivial (int count, ace_t *entries) > ^ > ..snip.. > > It seems that acl_ace_nontrivial is pure, so mark it as such. > For acl_nontrivial though, the AIX version isn't pure, so disable > the warning instead. I'm not keen on squashing the warning for all > versions of the function, but applying the attribute selectively > doesn't feel right either. Maybe I'm missing the nicest solution > entirely? On AIX acl_last() is just a macro that reads mem and so gcc should see it as such, so therefore we might be able to mark acl_nontrivial() as pure also? thanks, Pádraig.
Re: [PATCH] acl: Address pure attribute errors with gcc 4.9
On 1 Jun 2014 21:16, "Pádraig Brady" wrote: > > On 06/01/2014 09:42 AM, Ben Walton wrote: > > * lib/acl-internal.h (acl_ace_nontrivial): Apply pure attribute > > * lib/file-has-acl.c: Disable pure attribute error. > > - The AIX version of acl_nontrivial isn't pure while other > > versions are. We cannot apply the attribute globally. > > > > Signed-off-by: Ben Walton > > --- > > When building coreutils 8.22 on Solaris 10 x86, gcc 4.9.0 emits: > > > > ..snip.. > > b/file-has-acl.c: In function 'acl_nontrivial': > > lib/file-has-acl.c:133:1: error: function might be candidate for attribute 'pure' [-Werror=suggest-attribute=pure] > > acl_nontrivial (int count, aclent_t *entries) > > ^ > > lib/file-has-acl.c: In function 'acl_ace_nontrivial': > > lib/file-has-acl.c:164:1: error: function might be candidate for attribute 'pure' [-Werror=suggest-attribute=pure] > > acl_ace_nontrivial (int count, ace_t *entries) > > ^ > > ..snip.. > > > > It seems that acl_ace_nontrivial is pure, so mark it as such. > > For acl_nontrivial though, the AIX version isn't pure, so disable > > the warning instead. I'm not keen on squashing the warning for all > > versions of the function, but applying the attribute selectively > > doesn't feel right either. Maybe I'm missing the nicest solution > > entirely? > > On AIX acl_last() is just a macro that reads mem > and so gcc should see it as such, so therefore we might > be able to mark acl_nontrivial() as pure also? If that's the case, then I fully support marking it explicitly to. Masking the error for any future changes with the pragma seems like the option to choose only when there isn't a better one. I'll resubmit. Thanks -Ben
Re: [PATCH] lib/rename.c: Conditionally define the out label
On 06/01/2014 09:34 AM, Ben Walton wrote: > * Avoid possible compiler warnings/errors by defining the out label > only when it may be accessed. > > Signed-off-by: Ben Walton > --- > > Hi All, > > When building coreutils 8.22 on Solaris with -Werror=unused-label, the build > fails with: > > lib/rename.c: In function 'rpl_rename': > lib/rename.c:465:2: error: label 'out' defined but not used > [-Werror=unused-label] > out: > ^ Are you building from a git checkout repo? Otherwise you should have to configure --enable-gcc-warnings to get -Werror enabled? > I think this should make the compiler happier. Feel free to suggest better > solutions though. I'm not sure this is the best way to handle it. > > lib/rename.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/lib/rename.c b/lib/rename.c > index 2116028..9c507c2 100644 > --- a/lib/rename.c > +++ b/lib/rename.c > @@ -462,7 +462,14 @@ rpl_rename (char const *src, char const *dst) > >ret_val = rename (src_temp, dst_temp); >rename_errno = errno; > + > +# if (RENAME_TRAILING_SLASH_SOURCE_BUG || RENAME_DEST_EXISTS_BUG\ > + || RENAME_HARD_LINK_BUG) > + /* Avoid compiler warnings about unused labels. Only > + create this label if it will be used. */ > out: > +# endif > + Note one can mark a label as possibly unused like: out: _GL_UNUSED; That's supported on all gcc, and newer g++ since https://gcc.gnu.org/ml/gcc-patches/2009-05/msg01897.html So to support compiling with older g++ one could: out: #if (!defined __cplusplus) || __GNUC__ >= ? || (__GNUC__ == ? && __GNUC_MINOR__ >= ?) _GL_UNUSED; #endif With the ?s filled in as appropriate. thanks, Pádraig.
Re: [PATCH] lib/rename.c: Conditionally define the out label
On 06/01/2014 09:59 PM, Pádraig Brady wrote: > On 06/01/2014 09:34 AM, Ben Walton wrote: >> * Avoid possible compiler warnings/errors by defining the out label >> only when it may be accessed. >> >> Signed-off-by: Ben Walton >> --- >> >> Hi All, >> >> When building coreutils 8.22 on Solaris with -Werror=unused-label, the build >> fails with: >> >> lib/rename.c: In function 'rpl_rename': >> lib/rename.c:465:2: error: label 'out' defined but not used >> [-Werror=unused-label] >> out: >> ^ > > Are you building from a git checkout repo? > Otherwise you should have to configure --enable-gcc-warnings > to get -Werror enabled? > >> I think this should make the compiler happier. Feel free to suggest better >> solutions though. I'm not sure this is the best way to handle it. >> >> lib/rename.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/lib/rename.c b/lib/rename.c >> index 2116028..9c507c2 100644 >> --- a/lib/rename.c >> +++ b/lib/rename.c >> @@ -462,7 +462,14 @@ rpl_rename (char const *src, char const *dst) >> >>ret_val = rename (src_temp, dst_temp); >>rename_errno = errno; >> + >> +# if (RENAME_TRAILING_SLASH_SOURCE_BUG || RENAME_DEST_EXISTS_BUG\ >> + || RENAME_HARD_LINK_BUG) >> + /* Avoid compiler warnings about unused labels. Only >> + create this label if it will be used. */ >> out: >> +# endif >> + > > Note one can mark a label as possibly unused like: > > out: _GL_UNUSED; > > That's supported on all gcc, and newer g++ since > https://gcc.gnu.org/ml/gcc-patches/2009-05/msg01897.html > So to support compiling with older g++ one could: > > out: > #if (!defined __cplusplus) || __GNUC__ >= ? > || (__GNUC__ == ? && __GNUC_MINOR__ >= ?) > _GL_UNUSED; > #endif > > With the ?s filled in as appropriate. Actually the above should be abstracted away from rename.c, So you can instead do: out: _GL_UNUSED_LABEL I'll apply the following to gnulib-common.m4 to support that: +/* gcc supports the "unused" attribute on possibly unused labels, and + g++ has since version 4.5. */ +#if !defined __cplusplus || __GNUC__ > 4 \ +|| (__GNUC__ == 4 && __GNUC_MINOR__ >= 5) +# define _GL_UNUSED_LABEL _GL_UNUSED; +#else +# define _GL_UNUSED_LABEL +#endif thanks, Pádraig.
[PATCH] lib/acl-internal.h: Apply pure attribute to two functions
* lib/acl-internal.h (acl_nontrivial, acl_ace_nontrivial): Mark Signed-off-by: Ben Walton --- lib/acl-internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/acl-internal.h b/lib/acl-internal.h index fdffe64..b238006 100644 --- a/lib/acl-internal.h +++ b/lib/acl-internal.h @@ -174,14 +174,14 @@ extern int acl_access_nontrivial (acl_t); /* Return 1 if the given ACL is non-trivial. Return 0 if it is trivial, i.e. equivalent to a simple stat() mode. */ -extern int acl_nontrivial (int count, aclent_t *entries); +extern int acl_nontrivial (int count, aclent_t *entries) _GL_ATTRIBUTE_PURE; # ifdef ACE_GETACL /* Solaris 10 */ /* Test an ACL retrieved with ACE_GETACL. Return 1 if the given ACL, consisting of COUNT entries, is non-trivial. Return 0 if it is trivial, i.e. equivalent to a simple stat() mode. */ -extern int acl_ace_nontrivial (int count, ace_t *entries); +extern int acl_ace_nontrivial (int count, ace_t *entries) _GL_ATTRIBUTE_PURE; /* Definitions for when the built executable is executed on Solaris 10 (newer version) or Solaris 11. */ -- 1.9.1
Re: [PATCH] lib/rename.c: Conditionally define the out label
On Sun, Jun 1, 2014 at 9:59 PM, Pádraig Brady wrote: > On 06/01/2014 09:34 AM, Ben Walton wrote: >> * Avoid possible compiler warnings/errors by defining the out label >> only when it may be accessed. >> >> Signed-off-by: Ben Walton >> --- >> >> Hi All, >> >> When building coreutils 8.22 on Solaris with -Werror=unused-label, the build >> fails with: >> >> lib/rename.c: In function 'rpl_rename': >> lib/rename.c:465:2: error: label 'out' defined but not used >> [-Werror=unused-label] >> out: >> ^ > > Are you building from a git checkout repo? > Otherwise you should have to configure --enable-gcc-warnings > to get -Werror enabled? I configured with: $ /home/bwalton/opencsw/coreutils/trunk/work/solaris10-i386/build-isa-pentium_pro/coreutils-8.22/configure --prefix=/opt/csw --exec_prefix=/opt/csw --bindir=/opt/csw/bin --sbindir=/opt/csw/sbin --libexecdir= /opt/csw/libexec --datadir=/opt/csw/share --sysconfdir=/etc/opt/csw --sharedstatedir=/opt/csw/share --localstatedir=/var/opt/csw --libdir=/opt/csw/lib --infodir=/opt/csw/share/info --includedir=/opt/csw/includ e --mandir=/opt/csw/share/man --program-prefix=g --with-libintl-prefix=/opt/csw --with-libiconv-prefix=/opt/csw --enable-no-install-program=chcon,su --enable-install-program=hostname,arch > >> I think this should make the compiler happier. Feel free to suggest better >> solutions though. I'm not sure this is the best way to handle it. >> >> lib/rename.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/lib/rename.c b/lib/rename.c >> index 2116028..9c507c2 100644 >> --- a/lib/rename.c >> +++ b/lib/rename.c >> @@ -462,7 +462,14 @@ rpl_rename (char const *src, char const *dst) >> >>ret_val = rename (src_temp, dst_temp); >>rename_errno = errno; >> + >> +# if (RENAME_TRAILING_SLASH_SOURCE_BUG || RENAME_DEST_EXISTS_BUG\ >> + || RENAME_HARD_LINK_BUG) >> + /* Avoid compiler warnings about unused labels. Only >> + create this label if it will be used. */ >> out: >> +# endif >> + > > Note one can mark a label as possibly unused like: > > out: _GL_UNUSED; > > That's supported on all gcc, and newer g++ since > https://gcc.gnu.org/ml/gcc-patches/2009-05/msg01897.html > So to support compiling with older g++ one could: > > out: > #if (!defined __cplusplus) || __GNUC__ >= ? > || (__GNUC__ == ? && __GNUC_MINOR__ >= ?) > _GL_UNUSED; > #endif > > With the ?s filled in as appropriate. Ok, I hadn't noticed this ability. WIth your suggested followup (prerequisite), I think that it's a better solution. I'll resubmit. 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
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.
Re: [PATCH] lib/acl-internal.h: Apply pure attribute to two functions
On 06/01/2014 11:39 PM, Ben Walton wrote: > * lib/acl-internal.h (acl_nontrivial, acl_ace_nontrivial): Mark > > Signed-off-by: Ben Walton > --- > lib/acl-internal.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/acl-internal.h b/lib/acl-internal.h > index fdffe64..b238006 100644 > --- a/lib/acl-internal.h > +++ b/lib/acl-internal.h > @@ -174,14 +174,14 @@ extern int acl_access_nontrivial (acl_t); > > /* Return 1 if the given ACL is non-trivial. > Return 0 if it is trivial, i.e. equivalent to a simple stat() mode. */ > -extern int acl_nontrivial (int count, aclent_t *entries); > +extern int acl_nontrivial (int count, aclent_t *entries) _GL_ATTRIBUTE_PURE; > > # ifdef ACE_GETACL /* Solaris 10 */ > > /* Test an ACL retrieved with ACE_GETACL. > Return 1 if the given ACL, consisting of COUNT entries, is non-trivial. > Return 0 if it is trivial, i.e. equivalent to a simple stat() mode. */ > -extern int acl_ace_nontrivial (int count, ace_t *entries); > +extern int acl_ace_nontrivial (int count, ace_t *entries) _GL_ATTRIBUTE_PURE; > > /* Definitions for when the built executable is executed on Solaris 10 > (newer version) or Solaris 11. */ > Pushed. thanks, Pádraig.
Re: [PATCH] lib/rename.c: Conditionally define the out label
Pádraig Brady wrote: +# define _GL_UNUSED_LABEL _GL_UNUSED; Why is there a semicolon at the end of that macro definition? It might cause problems, no? _GL_UNUSED_LABEL is supposed to not affect the semantics of the program, but with a semicolon there it could change things.
Re: [PATCH] lib/rename.c: Conditionally define the out label
Paul Eggert wrote: Pádraig Brady wrote: +# define _GL_UNUSED_LABEL _GL_UNUSED; Why is there a semicolon at the end of that macro definition? I removed it just now (since I was syncing to Emacs and I couldn't stand seeing the typo there...).
Re: [PATCH] lib/rename.c: Conditionally define the out label
On 06/02/2014 12:53 AM, Paul Eggert wrote:
> Paul Eggert wrote:
>> Pádraig Brady wrote:
>>
>>> +# define _GL_UNUSED_LABEL _GL_UNUSED;
>>
>> Why is there a semicolon at the end of that macro definition?
>
> I removed it just now (since I was syncing to Emacs and I couldn't stand
> seeing the typo there...).
Well the ; is needed in C++ but optional in C.
I was worried about users leaving out the ; by mistake
and not noticing in the normal case of compiling in C.
I suppose one could contrive a change in behavior using something like
if (false)
label: _GL_UNUSED_LABEL puts("true");
That would output "true" always.
Better to give the compile error on C++ rather than that silent gotcha.
thanks,
Pádraig.
Re: [PATCH] lib/rename.c: Conditionally define the out label
Pádraig Brady wrote: Well the ; is needed in C++ but optional in C. I was worried about users leaving out the ; by mistake and not noticing in the normal case of compiling in C. But then the other case should be "# define _GL_UNUSED_LABEL ;", no? Otherwise a semicolon would be appended sometimes but not others, which cannot be right. My experience is that macros whose expansion ends in ";" tend to cause confusion, so I'd rather omit the ";" in both cases. But if it's going to be added in the "if" it should also be added in the "else".
Re: [PATCH] lib/rename.c: Conditionally define the out label
Another argument for omitting the semicolon is, suppose we add another label-related attribute later? _GL_HOT, say? Then, 'lab: _GL_HOT _GL_UNUSED_LABEL' would work, but 'lab: _GL_UNUSED_LABEL _GL_HOT' would not, and users would have to remember to always put _GL_UNUSED_LABEL last among all the other label-related _GL_whatever attributes; that wouldn't be good.
Re: [PATCH] lib/rename.c: Conditionally define the out label
On 06/02/2014 01:14 AM, Paul Eggert wrote: > Pádraig Brady wrote: >> Well the ; is needed in C++ but optional in C. >> I was worried about users leaving out the ; by mistake >> and not noticing in the normal case of compiling in C. > > But then the other case should be "# define _GL_UNUSED_LABEL ;", no? > Otherwise a semicolon would be appended sometimes but not others, which > cannot be right. Yes I should have added ; to the else > My experience is that macros whose expansion ends in ";" tend to cause > confusion, so I'd rather omit the ";" in both cases. Yes I agree, let's leave out the trailing ; thanks, Pádraig.
