cppcheck errors and gcc warning

[Arno Onken]

cppcheck version 1.58 reports the following errors on the latest
Savannah respository checkout of gnulib (last changeSat, 16 Mar 2013
05:13:46 +):

[lib/rpmatch.c:113]: (error) Memory leak: safe_pattern
[lib/regcomp.c:2848]: (error) Uninitialized variable: symb_table
[lib/regcomp.c:2847]: (error) Uninitialized variable: table_size
[lib/regcomp.c:2850]: (error) Uninitialized variable: table_size

Also, gcc version 4.7.2 reports a warning:

copy-file.c: In function 'qcopy_file_preserving':
copy-file.c:143:9: warning: ignoring return value of 'chown', declared
with attribute warn_unused_result [-Wunused-result]

Re: ::gettimeofday' has not been declared - on cygwin

[marco atzeri]

On 3/19/2013 7:34 AM, Paul Eggert wrote:

On 03/01/2013 08:40 PM in 
 marco 
atzeri wrote:


reverting the commit:

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=db61fd22822da6974c2ca4caa2975d74229a4c70

solves the issue.


Thanks for reporting that.  Most likely the problem is in the following
part of lib/sys_select.in.h.  My guess is that Cygwin is like OSF/1 and
Solaris 2.6 in that  (and perhaps sys/types.h) include sys/select.h
and we need to catch that, and delegate to the system's header in this case.
Can you please analyze or supply copies of the relevant include files on Cygwin,
so that we can add the appropriate "|| defined __CYGWIN__ && ..."  clauses
below?  We need to know under which circumstances, exactly, the system
 includes , and what macros the system 
defines to prevent double inclusion, and likewise for .

Thanks.


Hi Paul,
I uploaded a bzipped copy of
  /usr/include/sys
  /usr/include/cygwin

on
  http://matzeri.altervista.org/works/include/

for what I see  includes  but
none of the two include directly 

The chain seems:
sys/time.h:#include 
cygwin/sys_time.h:#include 

Regards
Marco

Re: ::gettimeofday' has not been declared - on cygwin

[Paul Eggert]

On 03/19/2013 03:15 AM, marco atzeri wrote:


   http://matzeri.altervista.org/works/include/

for what I see  includes  but
none of the two include directly 


I think I see an indirect inclusion.  Does the attached gnulib
patch fix things for you?  I'll CC: this to Tom Christensen, as
this is fallout from the Solaris 2.6 fixes in January and I'd
like the code to continue to work there too.

From bfd11c25219f5800465e67a01c580e4b9099d209 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 19 Mar 2013 09:08:47 -0700
Subject: [PATCH] sys_select: Port 2013-01-30 Solaris 2.6 fix to Cygwin.

Problem reported by Marco Atzeri in
.
* lib/sys_select.in.h [HAVE_SYS_SELECT_H && _CYGWIN_SYS_TIME_H]:
Simply delegate to the system  in this case too.
---
 ChangeLog   | 8 
 lib/sys_select.in.h | 5 -
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 98d74d3..24e44c2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2013-03-19  Paul Eggert  
+
+   sys_select: Port 2013-01-30 Solaris 2.6 fix to Cygwin.
+   Problem reported by Marco Atzeri in
+   .
+   * lib/sys_select.in.h [HAVE_SYS_SELECT_H && _CYGWIN_SYS_TIME_H]:
+   Simply delegate to the system  in this case too.
+
 2013-01-27  Jim Meyering  
 
prefix-gnulib-mk: give better diagnostics
diff --git a/lib/sys_select.in.h b/lib/sys_select.in.h
index 5a88d51..117442d 100644
--- a/lib/sys_select.in.h
+++ b/lib/sys_select.in.h
@@ -21,6 +21,7 @@
 
 /* On OSF/1 and Solaris 2.6,  and 
both include .
+   On Cygwin,  includes .
Simply delegate to the system's header in this case.  */
 #if (@HAVE_SYS_SELECT_H@\
  && ((defined __osf__ && defined _SYS_TYPES_H_ && defined _OSF_SOURCE) \
@@ -33,7 +34,9 @@
 # @INCLUDE_NEXT@ @NEXT_SYS_SELECT_H@
 
 #elif (@HAVE_SYS_SELECT_H@  \
-   && ((defined __osf__ && defined _SYS_TIME_H_ && defined _OSF_SOURCE) \
+   && (defined _CYGWIN_SYS_TIME_H   \
+   || (defined __osf__ && defined _SYS_TIME_H_  \
+   && defined _OSF_SOURCE)  \
|| (defined __sun && defined _SYS_TIME_H \
&& (! (defined _XOPEN_SOURCE || defined _POSIX_C_SOURCE) \
|| defined __EXTENSIONS__))) \
-- 
1.7.11.7

Re: cppcheck errors and gcc warning

[Paul Eggert]

Thanks.  The rpmatch.c issue is a true memory leak.

The three regcomp.c issues seem to be false alarms: that code
is not compiled for gnulib and the diagnostics suggest that there
may be a bug in cppcheck.

The copy-file.c issue is not a real bug but I suppose other people
will run into the false alarm so it's worth fixing.

I pushed the attached patch to try to address the two problems caught.
From 946440bdf504f580c8810aaddcab52655f8f3047 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 19 Mar 2013 09:30:58 -0700
Subject: [PATCH] copy-file, rpmatch: fix problems found by cppcheck

Reported by Arno Onken in
.
* lib/rpmatch.c (try): Fix memory leak.
* lib/copy-file.c: Include "ignore-value.h".
(qcopy_file_preserving): Ignore chown value.
* modules/copy-file (Depends-on): Add ignore-value.
---
 ChangeLog | 10 ++
 lib/copy-file.c   |  3 ++-
 lib/rpmatch.c |  5 -
 modules/copy-file |  1 +
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 98d74d3..cfa4751 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2013-03-19  Paul Eggert  
+
+   copy-file, rpmatch: fix problems found by cppcheck
+   Reported by Arno Onken in
+   .
+   * lib/rpmatch.c (try): Fix memory leak.
+   * lib/copy-file.c: Include "ignore-value.h".
+   (qcopy_file_preserving): Ignore chown value.
+   * modules/copy-file (Depends-on): Add ignore-value.
+
 2013-01-27  Jim Meyering  
 
prefix-gnulib-mk: give better diagnostics
diff --git a/lib/copy-file.c b/lib/copy-file.c
index 0d4dcd6..5660522 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -37,6 +37,7 @@
 #endif
 
 #include "error.h"
+#include "ignore-value.h"
 #include "safe-read.h"
 #include "full-write.h"
 #include "acl.h"
@@ -140,7 +141,7 @@ qcopy_file_preserving (const char *src_filename, const char 
*dest_filename)
 
 #if HAVE_CHOWN
   /* Preserve the owner and group.  */
-  chown (dest_filename, statbuf.st_uid, statbuf.st_gid);
+  ignore_value (chown (dest_filename, statbuf.st_uid, statbuf.st_gid));
 #endif
 
   /* Preserve the access permissions.  */
diff --git a/lib/rpmatch.c b/lib/rpmatch.c
index cf12523..ccb45f3 100644
--- a/lib/rpmatch.c
+++ b/lib/rpmatch.c
@@ -110,7 +110,10 @@ try (const char *response, const char *pattern, char 
**lastp, regex_t *re)
 return -1;
   /* Compile the pattern and cache it for future runs.  */
   if (regcomp (re, safe_pattern, REG_EXTENDED) != 0)
-return -1;
+{
+  free (safe_pattern);
+  return -1;
+}
   *lastp = safe_pattern;
 }
 
diff --git a/modules/copy-file b/modules/copy-file
index 1c50d55..2673ff0 100644
--- a/modules/copy-file
+++ b/modules/copy-file
@@ -13,6 +13,7 @@ error
 fstat
 full-write
 gettext-h
+ignore-value
 open
 quote
 safe-read
-- 
1.7.11.7

Re: ::gettimeofday' has not been declared - on cygwin

[marco atzeri]

On 3/19/2013 5:12 PM, Paul Eggert wrote:

On 03/19/2013 03:15 AM, marco atzeri wrote:


   http://matzeri.altervista.org/works/include/

for what I see  includes  but
none of the two include directly 


I think I see an indirect inclusion.  Does the attached gnulib
patch fix things for you?  I'll CC: this to Tom Christensen, as
this is fallout from the Solaris 2.6 fixes in January and I'd
like the code to continue to work there too.



it does not seem to work:

In file included from /usr/include/sys/select.h:23:0,
 from ../libgnu/sys/select.h:47,
 from /usr/include/cygwin/sys_time.h:13,
 from /usr/include/sys/time.h:28,
 from ../libgnu/sys/time.h:30,
 from /usr/include/sys/_default_fcntl.h:186,
 from /usr/include/sys/fcntl.h:3,
 from /usr/include/fcntl.h:14,
 from ../libgnu/fcntl.h:61,
 from ../../octave/liboctave/system/lo-sysdep.cc:34:
../libgnu/sys/time.h:411:1: error: '::gettimeofday' has not been declared
Makefile:9582: recipe for target 
`system/system_libsystem_la-lo-sysdep.lo' failed

cppcheck error in opendir.c

[Arno Onken]

cppcheck version 1.58 reports the following error on the latest
Savannah respository checkout of gnulib (last change Tue, 19 Mar 2013
17:47:12 +):

[lib/opendir.c:140]: (error) Mismatching allocation and deallocation: dirp

Re: cppcheck error in opendir.c

[Paul Eggert]

On 03/19/13 11:59, Arno Onken wrote:
> cppcheck version 1.58 reports the following error on the latest
> Savannah respository checkout of gnulib (last change Tue, 19 Mar 2013
> 17:47:12 +):
> 
> [lib/opendir.c:140]: (error) Mismatching allocation and deallocation: dirp

I don't see why this diagnostic was generated.
Maybe a bug in cppcheck?

Re: cppcheck error in opendir.c

[Arno Onken]

On 03/19/2013 08:57 PM, Paul Eggert wrote:
> On 03/19/13 11:59, Arno Onken wrote:
>> cppcheck version 1.58 reports the following error on the latest
>> Savannah respository checkout of gnulib (last change Tue, 19 Mar 2013
>> 17:47:12 +):
>>
>> [lib/opendir.c:140]: (error) Mismatching allocation and deallocation: dirp
> 
> I don't see why this diagnostic was generated.
> Maybe a bug in cppcheck?

Perhaps. I'll report it to the cppcheck tracker.

Thanks,
Arno

bug-gnulib@gnu.org

[Karl Berry]

Is there a portability reason to use 1>&2 vs. just >&2 to redirect to stderr?
I don't know of one, and don't see it mentioned in autoconf(Portable Shell).
The gnupload script currently uses both.
Just wondering.

k

bug-gnulib@gnu.org

[Eric Blake]

On 03/19/2013 03:20 PM, Karl Berry wrote:
> Is there a portability reason to use 1>&2 vs. just >&2 to redirect to stderr?
> I don't know of one, and don't see it mentioned in autoconf(Portable Shell).

As far as I know, all shells treat both as synonyms.  1>&2 is a bit more
explicit about what is happening, >&2 is faster to type.

> The gnupload script currently uses both.

Changing for consistency might be nice, but portability-wise, there is
no problem.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature