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