zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH 0/1] Fix -Wformat-overflow= warnings in zftp.c
@ 2019-03-23 10:11 Wesley Schwengle
  2019-03-23 10:11 ` [PATCH] Change handrolled strftime to strftime " Wesley Schwengle
  0 siblings, 1 reply; 9+ messages in thread
From: Wesley Schwengle @ 2019-03-23 10:11 UTC (permalink / raw)
  To: zsh-workers; +Cc: Wesley Schwengle

Hello all,

I was compiling zsh from master on my Debian testing box with cc 8.3.0
and I got the following warning:

zftp.c: In function ‘zfstats’:
zftp.c:1265:26: warning: ‘%02d’ directive writing between 2 and 11 bytes
into a region of size between 9 and 16 [-Wformat-overflow=]
sprintf(tmbuf, "%04d%02d%02d%02d%02d%02d",
^~~~
zftp.c:1265:21: note: directive argument in the range [-2147483647, 2147483647]
sprintf(tmbuf, "%04d%02d%02d%02d%02d%02d",
^~~~~~~~~~~~~~~~~~~~~~~~~~
zftp.c:1265:6: note: ‘sprintf’ output between 15 and 67 bytes into a
destination of size 20
sprintf(tmbuf, "%04d%02d%02d%02d%02d%02d",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tm->tm_year + 1900, tm->tm_mon+1, tm->tm_mday,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tm->tm_hour, tm->tm_min, tm->tm_sec);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I made a small patch that resolves the warning.

Cheers,
Wesley

Wesley Schwengle (1):
  Change handrolled strftime to strftime in zftp.c

 Src/Modules/zftp.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

-- 
2.21.0.196.g041f5ea1cf


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-23 10:11 [PATCH 0/1] Fix -Wformat-overflow= warnings in zftp.c Wesley Schwengle
@ 2019-03-23 10:11 ` Wesley Schwengle
  2019-03-25 12:13   ` Jun T
  0 siblings, 1 reply; 9+ messages in thread
From: Wesley Schwengle @ 2019-03-23 10:11 UTC (permalink / raw)
  To: zsh-workers; +Cc: Wesley Schwengle

From: Wesley Schwengle <wesley@schwengle.net>

Compiling zsh on Debian testing/unstable with cc (Debian 8.3.0-2) 8.3.0
emits a -Wformat-overflow= warning on line 1265 in zftp.c. Replace the
code with strftime() resolves it.

Signed-off-by: Wesley Schwengle <wesley@schwengle.net>
---
 Src/Modules/zftp.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
index 4aaa1f072..e0789216a 100644
--- a/Src/Modules/zftp.c
+++ b/Src/Modules/zftp.c
@@ -1257,14 +1257,7 @@ zfstats(char *fnam, int remote, off_t *retsize, char **retmdtm, int fd)
 	if (retmdtm) {
 	    /* use gmtime() rather than localtime() for consistency */
 	    tm = gmtime(&statbuf.st_mtime);
-	    /*
-	     * FTP format for data is YYYYMMDDHHMMSS
-	     * Using tm directly is easier than worrying about
-	     * incompatible strftime()'s.
-	     */
-	    sprintf(tmbuf, "%04d%02d%02d%02d%02d%02d",
-		    tm->tm_year + 1900, tm->tm_mon+1, tm->tm_mday,
-		    tm->tm_hour, tm->tm_min, tm->tm_sec);
+	    strftime(tmbuf, 15, "%Y%m%d%H%M%S", tm);
 	    mt = ztrdup(tmbuf);
 	}
     }
-- 
2.21.0.196.g041f5ea1cf


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-23 10:11 ` [PATCH] Change handrolled strftime to strftime " Wesley Schwengle
@ 2019-03-25 12:13   ` Jun T
  2019-03-25 12:42     ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Jun T @ 2019-03-25 12:13 UTC (permalink / raw)
  To: zsh-workers


> 2019/03/23 19:11, Wesley Schwengle <wesleyschwengle@gmail.com> wrote:
> 
> +	    strftime(tmbuf, 15, "%Y%m%d%H%M%S", tm);

zsh has its own ztrftime() (which works on systems without strftime())
and we can use it here.

I tried it and of course it worked, but I noticed another (unrelated)
problem. If I build zsh with --enable-zsh-debug, and test on my Mac,

% zmodload zsh/zftp
% zftp local flame
 zftp.c:2519: Shell compiled with wrong off_t size

On Darwin (and 64 bit Linux) both long and off_t are 64 bit. In
configure.ac, line 975 and below, it says OFF_T_IS_64_BIT is
defined only if LONG_IS_64_BIT is NOT defined (i.e., long is 32 bit)
and off_t is 64 bit. But in zftp.c, line 2518 (before patch),


#ifdef OFF_T_IS_64_BIT
        printf("%s %s\n", output64(sz), mt);
#else
        DPUTS(sizeof(sz) > 4, "Shell compiled with wrong off_t size");
        printf("%ld %s\n", (long)sz, mt);
#endif


Should we replace "#ifdef OFF_T_IS_64_BIT" by "#ifndef OFF_T_IS_64_BIT"?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 12:13   ` Jun T
@ 2019-03-25 12:42     ` Peter Stephenson
  2019-03-25 13:07       ` Jun T.
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2019-03-25 12:42 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2019-03-25 at 21:13 +0900, Jun T wrote:
> #ifdef OFF_T_IS_64_BIT
>         printf("%s %s\n", output64(sz), mt);
> #else
>         DPUTS(sizeof(sz) > 4, "Shell compiled with wrong off_t size");
>         printf("%ld %s\n", (long)sz, mt);
> #endif
> 
> 
> Should we replace "#ifdef OFF_T_IS_64_BIT" by "#ifndef OFF_T_IS_64_BIT"?

The obvious test would be

#ifdef ZSH_64_BIT_TYPE

which is how we know output64 is defined.  That's used in some other case.

Cheers
pws



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 12:42     ` Peter Stephenson
@ 2019-03-25 13:07       ` Jun T.
  2019-03-25 13:32         ` Jun T.
  2019-03-25 14:36         ` Peter Stephenson
  0 siblings, 2 replies; 9+ messages in thread
From: Jun T. @ 2019-03-25 13:07 UTC (permalink / raw)
  To: zsh-workers


> 2019/03/25 21:42, Peter Stephenson <p.stephenson@samsung.com> wrote:
>>  
>> Should we replace "#ifdef OFF_T_IS_64_BIT" by "#ifndef OFF_T_IS_64_BIT"?
> 
> The obvious test would be
> 
> #ifdef ZSH_64_BIT_TYPE

But ZSH_64_BIT_TYPE is defined only if long is 32 bit
so we still get 'Shell compiled with wrong off_t size'.
Do you mean "#ifndef ZSH_64_BIT_TYPE"?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 13:07       ` Jun T.
@ 2019-03-25 13:32         ` Jun T.
  2019-03-25 14:36         ` Peter Stephenson
  1 sibling, 0 replies; 9+ messages in thread
From: Jun T. @ 2019-03-25 13:32 UTC (permalink / raw)
  To: zsh-workers


Sorry, the problem is not the OFF_T_IS_64_BIT but the comparison
in DPUTS().

I will push Wesley Schwengle's patch (with strftime --> ztrftime)
without the following because the two are no related.


diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
index 4aaa1f072..b3fbeae17 100644
--- a/Src/Modules/zftp.c
+++ b/Src/Modules/zftp.c
@@ -2518,7 +2518,8 @@ zftp_local(UNUSED(char *name), char **args, int flags)
 #ifdef OFF_T_IS_64_BIT
 	printf("%s %s\n", output64(sz), mt);
 #else
-	DPUTS(sizeof(sz) > 4, "Shell compiled with wrong off_t size");
+	DPUTS(sizeof(sz) > sizeof(long), 
+	      "Shell compiled with wrong off_t size");
 	printf("%ld %s\n", (long)sz, mt);
 #endif
 	zsfree(mt);





^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 13:07       ` Jun T.
  2019-03-25 13:32         ` Jun T.
@ 2019-03-25 14:36         ` Peter Stephenson
  2019-03-25 15:06           ` Jun T.
  2019-03-25 15:08           ` Peter Stephenson
  1 sibling, 2 replies; 9+ messages in thread
From: Peter Stephenson @ 2019-03-25 14:36 UTC (permalink / raw)
  To: zsh-workers

> On 25 March 2019 at 13:07 "Jun T." <takimoto-j@kba.biglobe.ne.jp> wrote:
> > 2019/03/25 21:42, Peter Stephenson <p.stephenson@samsung.com> wrote:
> >>  
> >> Should we replace "#ifdef OFF_T_IS_64_BIT" by "#ifndef OFF_T_IS_64_BIT"?
> > 
> > The obvious test would be
> > 
> > #ifdef ZSH_64_BIT_TYPE
> 
> But ZSH_64_BIT_TYPE is defined only if long is 32 bit
> so we still get 'Shell compiled with wrong off_t size'.
> Do you mean "#ifndef ZSH_64_BIT_TYPE"?

I don't see how that could work since output64() wouldn't exist.

I think this means the error message in the other branch is now redundant --- if
long is a 64-bit value it's perfectly OK to use that code.  Could change the DPUTS
check to check sizeof(sz) for that purpose instead?

pws

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 14:36         ` Peter Stephenson
@ 2019-03-25 15:06           ` Jun T.
  2019-03-25 15:08           ` Peter Stephenson
  1 sibling, 0 replies; 9+ messages in thread
From: Jun T. @ 2019-03-25 15:06 UTC (permalink / raw)
  To: zsh-workers


> 2019/03/25 23:36, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:

> Could change the DPUTS
> check to check sizeof(sz) for that purpose instead?

Yes, as in my previous post (mailing list has some delay).

A similar fix for stat.c is
worker/20823
commit d7c13fb2c3b1b014acde9c1cb17a1e34239b9751


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Change handrolled strftime to strftime in zftp.c
  2019-03-25 14:36         ` Peter Stephenson
  2019-03-25 15:06           ` Jun T.
@ 2019-03-25 15:08           ` Peter Stephenson
  1 sibling, 0 replies; 9+ messages in thread
From: Peter Stephenson @ 2019-03-25 15:08 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2019-03-25 at 14:36 +0000, Peter Stephenson wrote:
> > 
> > On 25 March 2019 at 13:07 "Jun T." <takimoto-j@kba.biglobe.ne.jp> wrote:
> > > 
> > > 2019/03/25 21:42, Peter Stephenson <p.stephenson@samsung.com> wrote:
> > > > 
> > > >  
> > > > Should we replace "#ifdef OFF_T_IS_64_BIT" by "#ifndef OFF_T_IS_64_BIT"?
> > > The obvious test would be
> > > 
> > > #ifdef ZSH_64_BIT_TYPE
> > But ZSH_64_BIT_TYPE is defined only if long is 32 bit
> > so we still get 'Shell compiled with wrong off_t size'.
> > Do you mean "#ifndef ZSH_64_BIT_TYPE"?
> I don't see how that could work since output64() wouldn't exist.
> 
> I think this means the error message in the other branch is now
> redundant --- if long is a 64-bit value it's perfectly OK to use that
> code.  Could change the DPUTS check to check sizeof(sz) for that
> purpose instead?

...which you've now done, so this should be fine.

pws

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-03-25 15:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-23 10:11 [PATCH 0/1] Fix -Wformat-overflow= warnings in zftp.c Wesley Schwengle
2019-03-23 10:11 ` [PATCH] Change handrolled strftime to strftime " Wesley Schwengle
2019-03-25 12:13   ` Jun T
2019-03-25 12:42     ` Peter Stephenson
2019-03-25 13:07       ` Jun T.
2019-03-25 13:32         ` Jun T.
2019-03-25 14:36         ` Peter Stephenson
2019-03-25 15:06           ` Jun T.
2019-03-25 15:08           ` Peter Stephenson

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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).