Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
Pavel Raiskup prais...@redhat.com ha escrit: It was just sugestion as tar uses -Werror by default since this commit: Ouch! A very unfortunate move, I must say. Regards, Sergey
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
On 02/20/2013 05:15 AM, Sergey Poznyakoff wrote:
Pavel Raiskup prais...@redhat.com ha escrit:
It was just sugestion as tar uses -Werror by default since this commit:
Ouch! A very unfortunate move, I must say.
It works well in other projects, such as coreutils: 'make' uses -Werror
only for developers, who check out from git. 'make' does not use -Werror
for ordinary builds from tarballs. Developers are assumed to have up-to-date
systems where -Werror is appropriate.
I'm surprised you didn't run into this. What's your workflow like? Don't
you have .git subdirectories?
The problem for regcomp.c was due to an interaction between a recent change
to the regex code and the particular set of warning options used for GNU tar.
I fixed it with the first attached patch. The other minor problems seem worth
fixing too, so I fixed them with the second attached patch.
From c9bfcaeb8f786531d16e3db613ccb99b36d7f83d Mon Sep 17 00:00:00 2001
From: Paul Eggert egg...@cs.ucla.edu
Date: Wed, 20 Feb 2013 07:50:59 -0800
Subject: [PATCH] regex: ignore old-style-definition warnings
* lib/regex.c: Add pragma to ignore these warnings.
Problem reported for GNU tar by Pavel Raiskup.
---
ChangeLog | 6 ++
lib/regex.c | 1 +
2 files changed, 7 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index a5b6b13..e66ca4c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2013-02-20 Paul Eggert egg...@cs.ucla.edu
+
+ regex: ignore old-style-definition warnings
+ * lib/regex.c: Add pragma to ignore these warnings.
+ Problem reported for GNU tar by Pavel Raiskup.
+
2013-02-19 Paul Eggert egg...@cs.ucla.edu
getcwd: support coreutils better
diff --git a/lib/regex.c b/lib/regex.c
index ca40e6e..361f763 100644
--- a/lib/regex.c
+++ b/lib/regex.c
@@ -24,6 +24,7 @@
# pragma GCC diagnostic ignored -Wsuggest-attribute=pure
# endif
# if (__GNUC__ == 4 3 = __GNUC_MINOR__) || 4 __GNUC__
+# pragma GCC diagnostic ignored -Wold-style-definition
# pragma GCC diagnostic ignored -Wtype-limits
# endif
#endif
--
1.7.11.7
From b38dd65a99715ae25d57d13926aae41c2c0609d9 Mon Sep 17 00:00:00 2001
From: Paul Eggert egg...@cs.ucla.edu
Date: Wed, 20 Feb 2013 08:09:38 -0800
Subject: [PATCH] tar: remove lint
* lib/wordsplit.c (_wsplit_error): Mark with printf attribute.
(expvar): Use defstr to pacify GCC.
* src/system.c (xexec): Now _Noreturn, to pacify GCC.
(run_decompress_program): Add cast to pacify GCC.
(sys_exec_command, sys_exec_info_script, sys_exec_checkpoint_script):
Remove unused variables.
---
lib/wordsplit.c | 7 +--
src/system.c| 24
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index bd5d59d..75faf9e 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -61,7 +61,7 @@ _wsplt_alloc_die (struct wordsplit *wsp)
abort ();
}
-static void
+static void __attribute__ ((__format__ (__printf__, 1, 2)))
_wsplt_error (const char *fmt, ...)
{
va_list ap;
@@ -795,7 +795,10 @@ expvar (struct wordsplit *wsp, const char *str, size_t len,
else
value = ;
}
+
/* FIXME: handle defstr */
+ (void) defstr;
+
if (value)
{
if (flg _WSNF_QUOTE)
@@ -1461,7 +1464,7 @@ wordsplit_process_list (struct wordsplit *wsp, size_t
start)
}
int
-wordsplit_len (const char *command, size_t length, struct wordsplit *wsp,
+wordsplit_len (const char *command, size_t length, struct wordsplit *wsp,
int flags)
{
int rc;
diff --git a/src/system.c b/src/system.c
index 6adcbf0..e1fd263 100644
--- a/src/system.c
+++ b/src/system.c
@@ -23,7 +23,7 @@
#include signal.h
#include wordsplit.h
-static void
+static _Noreturn void
xexec (const char *cmd)
{
struct wordsplit ws;
@@ -329,7 +329,7 @@ sys_child_open_for_compress (void)
int child_pipe[2];
pid_t grandchild_pid;
pid_t child_pid;
-
+
xpipe (parent_pipe);
child_pid = xfork ();
@@ -474,7 +474,7 @@ run_decompress_program (void)
ws.ws_env = (const char **) environ;
ws.ws_offs = 1;
-
+
for (p = first_decompress_program (i); p; p = next_decompress_program (i))
{
if (prog)
@@ -490,8 +490,8 @@ run_decompress_program (void)
wsflags |= WRDSF_REUSE;
memmove(ws.ws_wordv, ws.ws_wordv + ws.ws_offs,
sizeof(ws.ws_wordv[0])*ws.ws_wordc);
- ws.ws_wordv[ws.ws_wordc] = -d;
- prog = p;
+ ws.ws_wordv[ws.ws_wordc] = (char *) -d;
+ prog = p;
execvp (ws.ws_wordv[0], ws.ws_wordv);
ws.ws_wordv[ws.ws_wordc] = NULL;
}
@@ -726,8 +726,7 @@ int
sys_exec_command (char *file_name, int typechar, struct tar_stat_info *st)
{
int p[2];
- char *argv[4];
-
+
xpipe (p);
pipe_handler = signal (SIGPIPE, SIG_IGN);
global_pid = xfork ();
@@ -787,7 +786,6 @@ int
sys_exec_info_script (const char **archive_name, int volume_number)
{
pid_t pid;
- char *argv[4];
char uintbuf[UINTMAX_STRSIZE_BOUND];
int p[2];
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
On Wed, 2013-02-20 at 14:42 +0300, Sergey Poznyakoff wrote:
Yes, maybe not complete list of warnings/errors:
Obviously, you use -Werror, which I'd rather recommend against.
Making something like
It was just sugestion as tar uses -Werror by default since this commit:
http://git.savannah.gnu.org/cgit/tar.git/commit/?id=cbc51277aa4de1f41434ba073f2e4546ead63005
regexec.c:586:1: error: old-style function definition
[-Werror=old-style-definition]
an error is way too stringent. Warnings are warnings, nothing more.
I know, but it just complicates tar's building from git repository now (and its
development then). Everybody must use
./configure --disable-gcc-warnings.
On Wed, 2013-02-20 at 15:09 +0300, Sergey Poznyakoff wrote:
Pavel Raiskup prais...@redhat.com ha escrit:
Do not call wordsplit on multiple places when not necessary.
(run_decompress_program): Do not call wordsplit - rather reuse xexec.
That's wrong. The intent of the function is to try all matching
decompressors and find the one which works. Xexec cannot be used here
because it exits if exec fails.
[PATCH] tar: simplify code in system.c
Oh, I forgot about it, thanks for correcting me. What about something like
that ^^^ ?
In the this approach, there I addressed also fix for #if{,n}def MSDOS.
Pavel
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
Hi Paul, Developers are assumed to have up-to-date systems where -Werror is appropriate. Frankly, I cannot imagine a situation where treaing a warning as an error can be appropriate. That's why I consider it unfortunate. I'm surprised you didn't run into this. That's quite simple. Configure enables -Werror only if gcc's version is 4.6 or newer. Mine is not. Regards, Sergey
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
Pavel Raiskup prais...@redhat.com ha escrit: It seems that it comes from mailutils (but not mentioned in your patch - not in mailutils) Actually it is not. It comes from one of my projects: https://puszcza.gnu.org.ua/projects/grecs and I synchronize it with mailutils from time to time. In the future I plan to include grecs as a submodule to Mailutils, so the duplication could be removed in that particular case. Wordsplit has been written so that it can be easily incorporated into another projects, without the need of pulling any additional dependencies. I have ran Coverity to scan for the new defects and it seems that there are three very small defects worth to fix: Thanks a lot, I'll fix these. Another problem is that I'm unable to compile the new code with './configure make'; Why? Do you get any errors? Regards, Sergey
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
Pavel Raiskup prais...@redhat.com ha escrit: Isn't that nice candidate for gnulib then? Yes, quite probably. I haven't yet thought about it, thanks for the suggestion :) Yes, maybe not complete list of warnings/errors: Obviously, you use -Werror, which I'd rather recommend against. Making something like regexec.c:586:1: error: old-style function definition [-Werror=old-style-definition] an error is way too stringent. Warnings are warnings, nothing more. Regards, Sergey
Re: [Bug-tar] [PATCH 0/1] Notes for wordsplit (was: Allow tar to usecommand for compressing consisiting of multiple words.)
Sorry for trashing CC list, recovering.. It seems that it comes from mailutils (but not mentioned in your patch - not in mailutils) Actually it is not. It comes from one of my projects: https://puszcza.gnu.org.ua/projects/grecs and I synchronize it with mailutils from time to time. In the future I plan to include grecs as a submodule to Mailutils, so the duplication could be removed in that particular case. Wordsplit has been written so that it can be easily incorporated into another projects, without the need of pulling any additional dependencies. Isn't that nice candidate for gnulib then? I feel this is quite useful piece of code. But it may be that gnulib project is too careful to incorporate this.. Another problem is that I'm unable to compile the new code with './configure make'; Why? Do you get any errors? Yes, maybe not complete list of warnings/errors: $ cat test.sh #!/bin/bash WHERE=`mktemp -d` pushd $WHERE git clone git://git.savannah.gnu.org/tar.git cd tar ./bootstrap autoreconf -vfi ./configure make -k popd [...] GEN ref-del.sed In file included from regex.c:72:0: regcomp.c: In function ‘rpl_re_set_syntax’: regcomp.c:262:1: error: old-style function definition [-Werror=old-style-definition] regcomp.c: In function ‘rpl_re_compile_fastmap’: regcomp.c:275:1: error: old-style function definition [-Werror=old-style-definition] regcomp.c: In function ‘rpl_regcomp’: regcomp.c:475:1: error: old-style function definition [-Werror=old-style-definition] regcomp.c: In function ‘rpl_regfree’: regcomp.c:663:1: error: old-style function definition [-Werror=old-style-definition] In file included from regex.c:73:0: regexec.c: In function ‘rpl_regexec’: regexec.c:224:1: error: old-style function definition [-Werror=old-style-definition] regexec.c: In function ‘rpl_re_match’: regexec.c:312:1: error: old-style function definition [-Werror=old-style-definition] regexec.c: In function ‘rpl_re_search’: regexec.c:325:1: error: old-style function definition [-Werror=old-style-definition] regexec.c: In function ‘rpl_re_match_2’: regexec.c:340:1: error: old-style function definition [-Werror=old-style-definition] regexec.c: In function ‘rpl_re_search_2’: regexec.c:354:1: error: old-style function definition [-Werror=old-style-definition] regexec.c: In function ‘rpl_re_set_registers’: regexec.c:586:1: error: old-style function definition [-Werror=old-style-definition] cc1: all warnings being treated as errors make[4]: *** [regex.o] Error 1 make[4]: Target `all-am' not remade because of errors. make[4]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/gnu' make[3]: *** [all-recursive] Error 1 make[3]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/gnu' make[2]: *** [all] Error 2 make[2]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/gnu' Making all in lib make[2]: Entering directory `/tmp/tmp.AHISKP3RET/tar/lib' GEN rmt-command.h make all-am make[3]: Entering directory `/tmp/tmp.AHISKP3RET/tar/lib' CC paxerror.o [...] CC xattr-at.o wordsplit.c: In function ‘_wsplt_error’: wordsplit.c:70:3: error: function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=missing-format-attribute] wordsplit.c: In function ‘expvar’: wordsplit.c:708:15: error: variable ‘defstr’ set but not used [-Werror=unused-but-set-variable] cc1: all warnings being treated as errors make[3]: *** [wordsplit.o] Error 1 make[3]: Target `all-am' not remade because of errors. make[3]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/lib' make[2]: *** [all] Error 2 make[2]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/lib' Making all in rmt make[2]: Entering directory `/tmp/tmp.AHISKP3RET/tar/rmt' make[2]: *** No rule to make target `../gnu/libgnu.a', needed by `rmt'. CC rmt.o make[2]: Target `all' not remade because of errors. make[2]: Leaving directory `/tmp/tmp.AHISKP3RET/tar/rmt' Making all in src make[2]: Entering directory `/tmp/tmp.AHISKP3RET/tar/src' CC buffer.o [...] Pavel
