* build issues when using pcc @ 2011-05-22 12:34 Szabolcs Nagy 2011-05-22 16:46 ` Jonathan Nieder 0 siblings, 1 reply; 4+ messages in thread From: Szabolcs Nagy @ 2011-05-22 12:34 UTC (permalink / raw) To: dash hello i compiled dash with pcc and found two issues: src/output.h the outc macro is not standard c: #define outc(c, file)<->((file)->nextc == (file)->end ? outcslow((c), (file)) : (*(file)->nextc = (c), (file)->nextc++)) type signature of the operands are int ? void : char* but the two sides of : should be compatible either use ... ? outcslow(), 0 : ... or ... ? outcslow() : ..., (void)0 otherwise a standard compliant compiler won't compile it the other issue is in the mkbuiltins: it assumes that the cpp does not produce empty lines with whitespace only the following line < $builtins sed '/^#/d; /^$/d' > $temp should be < $builtins sed '/^#/d; /^ *$/d' > $temp so empty lines with spaces are dropped as well (allowed by the standard after the preprocessing pass) (also it would be nice to include a configure script in the git repo, or don't use autoconf in the first place) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: build issues when using pcc 2011-05-22 12:34 build issues when using pcc Szabolcs Nagy @ 2011-05-22 16:46 ` Jonathan Nieder [not found] ` <20110522173900.GG6142@port70.net> 2011-07-07 7:22 ` Herbert Xu 0 siblings, 2 replies; 4+ messages in thread From: Jonathan Nieder @ 2011-05-22 16:46 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: dash Hi, Szabolcs Nagy wrote: > hello i compiled dash with pcc and found two issues: > > src/output.h the outc macro is not standard c: Thanks. I tried and succeeded in reproducing this first one by noticing that "gcc -pedantic" produces a warning. It also produces many other warnings --- for example, one about use of statement expressions in src/error.h. It might be nice to fix more of these. > the following line > < $builtins sed '/^#/d; /^$/d' > $temp > should be > < $builtins sed '/^#/d; /^ *$/d' > $temp > > so empty lines with spaces are dropped as well > (allowed by the standard after the preprocessing pass) What cpp do you use (so others can catch similar problems in the future)? Looking over the C standard, it seems that if cc -E were exactly carrying out translation phases 1-4 (which it isn't :)) then we would expect each comment to be transformed to a single space character. So this looks like a good fix for portability. > (also it would be nice to include a configure script in > the git repo, No, it would not be nice. > or don't use autoconf in the first place) The nice things automake offers for dash are support for cross-compilation, dependency detection using gcc -MD, and VPATH builds. I am a nobody, so my opinion does not mean much, but: I would be very happy to use a plain makefile that can do those things, too. In that imagined scheme, the configure script would stick to what it is best at --- detecting information about the platform and configuration. The configure script output could go in a makefile fragment that the makefile uses with an "include" directive (relying on sane defaults when it is not present). The git project is an example of this (though missing features like cross-compilation and VPATH builds). I'd be happy to help out with that (as an alternate build system at first). Thanks and hope that helps. -- >8 -- Subject: [OUTPUT] Make outc an inline function As "gcc -pedantic" warns, ISO C forbids conditional expressions with only one void side. So the (needslow ? slowpath() : fastpath) code for outc in the !USE_GLIBC_STDIO case might not be portable. More importantly, it's hard to read. Rip it out and replace it with an inline function which should generate the same code. Reported-by: Szabolcs Nagy <nsz@port70.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Not even compile-tested. src/output.h | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/output.h b/src/output.h index d123301..4632329 100644 --- a/src/output.h +++ b/src/output.h @@ -97,10 +97,21 @@ freestdout() #define OUTPUT_ERR 01 /* error occurred on output */ #ifdef USE_GLIBC_STDIO -#define outc(c, o) putc((c), (o)->stream) +static inline void outc(int ch, struct output *file) +{ + putc(ch, file->stream); +} #define doformat(d, f, a) vfprintf((d)->stream, (f), (a)) #else -#define outc(c, file) ((file)->nextc == (file)->end ? outcslow((c), (file)) : (*(file)->nextc = (c), (file)->nextc++)) +static inline void outc(int ch, struct output *file) +{ + if (file->nextc != file->end) { + *file->nextc = ch; + file->nextc++; + return; + } + outcslow(ch, file); +} #endif #define out1c(c) outc((c), out1) #define out2c(c) outcslow((c), out2) -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
[parent not found: <20110522173900.GG6142@port70.net>]
* Re: build issues when using pcc [not found] ` <20110522173900.GG6142@port70.net> @ 2011-05-22 17:41 ` Jonathan Nieder 0 siblings, 0 replies; 4+ messages in thread From: Jonathan Nieder @ 2011-05-22 17:41 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: dash Szabolcs Nagy wrote: > * Jonathan Nieder <jrnieder@gmail.com> [2011-05-22 11:46:05 -0500]: >> What cpp do you use (so others can catch similar problems in the >> future)? > > pcc uses its own preprocessor Ah, sloppy me. I stupidly read pcc as "ppc" but you are talking about http://pcc.ludd.ltu.se/ Will check it out; thank you. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: build issues when using pcc 2011-05-22 16:46 ` Jonathan Nieder [not found] ` <20110522173900.GG6142@port70.net> @ 2011-07-07 7:22 ` Herbert Xu 1 sibling, 0 replies; 4+ messages in thread From: Herbert Xu @ 2011-07-07 7:22 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Szabolcs Nagy, dash On Sun, May 22, 2011 at 04:46:05PM +0000, Jonathan Nieder wrote: > > Subject: [OUTPUT] Make outc an inline function > > As "gcc -pedantic" warns, ISO C forbids conditional expressions with > only one void side. So the (needslow ? slowpath() : fastpath) code > for outc in the !USE_GLIBC_STDIO case might not be portable. > > More importantly, it's hard to read. Rip it out and replace it > with an inline function which should generate the same code. > > Reported-by: Szabolcs Nagy <nsz@port70.net> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Patch applied. I've also added this one-liner on top to eliminate an unnecessary promotion caused by this patch. commit 155b0b518f485b8b5cb574cd2f0dc936c3fd1902 Author: Herbert Xu <herbert@gondor.apana.org.au> Date: Thu Jul 7 15:22:03 2011 +0800 [BUILTIN] Eliminate unnecessary promotion in echocmd The patch to make outc into an inline function created an unnecessary promotion in echocmd due to its use of char vs. the int used by outc. This patch changes echocmd to use int instead. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> diff --git a/ChangeLog b/ChangeLog index ade43e9..2093fc7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 2011-07-07 Herbert Xu <herbert@gondor.apana.org.au> * Optimize dash -c "command" to avoid a fork. + * Eliminate unnecessary promotion in echocmd. 2011-05-22 Jonathan Nieder <jrnieder@gmail.com> diff --git a/src/bltin/printf.c b/src/bltin/printf.c index b0c3774..893295c 100644 --- a/src/bltin/printf.c +++ b/src/bltin/printf.c @@ -443,7 +443,7 @@ echocmd(int argc, char **argv) } do { - char c; + int c; nonl += conv_escape_str(*argv); outstr(stackblock(), outs); Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-07 7:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-22 12:34 build issues when using pcc Szabolcs Nagy 2011-05-22 16:46 ` Jonathan Nieder [not found] ` <20110522173900.GG6142@port70.net> 2011-05-22 17:41 ` Jonathan Nieder 2011-07-07 7:22 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).