Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bleadperl v5.27.0-228-g34f8e9c776 breaks JREFIOR/Finance-StockAccount-0.01.tar.gz #16046

Closed
p5pRT opened this issue Jun 26, 2017 · 13 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Jun 26, 2017

Migrated from rt.perl.org#131659 (status was 'resolved')

Searchable as RT131659$

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @andk

Kudos to Slaven who digged it up!

bisect


commit 34f8e9c
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed May 31 11​:59​:48 2017 +0100

  Perl_sv_vcatpvfn_flags​: simplify float_need calc

diagnostics


t/realization.t ............ 1/? panic​: snprintf buffer overflow at /tmp/loop_over_bdir-114
42-Bx0l84/Finance-StockAccount-0.01-0/blib/lib/Finance/StockAccount/Realization.pm line 173
.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 16.
t/realization.t ............ Dubious, test returned 255 (wstat 65280, 0xff00)
All 16 subtests passed
[...]
t/allactivityregression.t (Wstat​: 65280 Tests​: 19 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: No plan found in TAP output
t/realization.t (Wstat​: 65280 Tests​: 16 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: No plan found in TAP output
t/set.t (Wstat​: 65280 Tests​: 22 Failed​: 0)
  Non-zero exit status​: 255
  Parse errors​: No plan found in TAP output
Files=13, Tests=201, 4 wallclock secs ( 0.04 usr 0.12 sys + 0.94 cusr 0.20 csys = 1.30
CPU)
Result​: FAIL
Failed 3/13 test programs. 0/201 subtests failed.

perl -V


Summary of my perl5 (revision 5 version 27 subversion 1) configuration​:
  Commit id​: 34f8e9c
  Platform​:
  osname=linux
  osvers=4.9.0-2-amd64
  archname=x86_64-linux-thread-multi-ld
  uname='linux k93msid 4.9.0-2-amd64 #1 smp debian 4.9.18-1 (2017-03-30) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4 -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Duseithreads -Duselongdouble -DDEBUGGING=-g'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=define
  uselongdouble=define
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
  optimize='-O2 -g'
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='6.3.0 20170406'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='long double'
  nvsize=16
  Off_t='off_t'
  lseeksize=8
  alignbytes=16
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/6/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.24.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.24'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  MULTIPLICITY
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_IMPLICIT_CONTEXT
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_ITHREADS
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_LONG_DOUBLE
  USE_PERLIO
  USE_PERL_ATOF
  USE_REENTRANT_API
  Built under linux
  Compiled at Jun 26 2017 19​:29​:40
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4/lib/site_perl/5.27.1/x86_64-linux-thread-multi-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4/lib/site_perl/5.27.1
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4/lib/5.27.1/x86_64-linux-thread-multi-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4/lib/5.27.1

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @jkeenan

On Mon, 26 Jun 2017 20​:04​:36 GMT, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Kudos to Slaven who digged it up!

bisect
------
commit 34f8e9c
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed May 31 11​:59​:48 2017 +0100

Perl_sv_vcatpvfn_flags​: simplify float_need calc

diagnostics
-----------
t/realization.t ............ 1/? panic​: snprintf buffer overflow at
/tmp/loop_over_bdir-114
42-Bx0l84/Finance-StockAccount-0.01-
0/blib/lib/Finance/StockAccount/Realization.pm line 173
.
# Tests were run but no plan was declared and done_testing() was not
seen.
# Looks like your test exited with 255 just after 16.
t/realization.t ............ Dubious, test returned 255 (wstat 65280,
0xff00)
All 16 subtests passed
[...]
t/allactivityregression.t (Wstat​: 65280 Tests​: 19 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
t/realization.t (Wstat​: 65280 Tests​: 16 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
t/set.t (Wstat​: 65280 Tests​: 22 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
Files=13, Tests=201, 4 wallclock secs ( 0.04 usr 0.12 sys + 0.94
cusr 0.20 csys = 1.30
CPU)
Result​: FAIL
Failed 3/13 test programs. 0/201 subtests failed.

This is curious. When I try to install Finance​::StockAccount via 'cpanm' I get the same failures as reported here, viz., 3 instances of this error​:

#####
panic​: snprintf buffer overflow at /home/jkeenan/.cpanm/work/1498508153.29549/Finance-StockAccount-0.01/blib/lib/Finance/StockAccount/Realization.pm line 173.
#####

... one for each of t/allactivityregression.t, t/realization.t, and t/set.t.

Now suppose that (a) I change into the .cpanm build directory and try to run the tests with 'prove'​:

#####
prove -I/home/jkeenan/testing/blead/lib -b t/*.t
#####

All tests PASS. But, of course, there's a cheat here. I should have been running with the 'prove' that was built in my 'blead' version of perl -- not the 'prove' that was installed with perl-5.26.0. When I do the right thing​:

#####
/home/jkeenan/testing/blead/bin/prove -I/home/jkeenan/testing/blead/lib -b t/*.t
#####

... I get the same panics, e.g.​:

#####
t/allactivityregression.t .. 1/? panic​: snprintf buffer overflow at /home/jkeenan/.cpanm/work/1498508153.29549/Finance-StockAccount-0.01/blib/lib/Finance/StockAccount/Realization.pm line 173.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 19.
t/allactivityregression.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
All 19 subtests passed
#####

Here is the relevant subroutine from Finance​::StockAccount​:

#####
ub string {
  my $self = shift;
  my $divestment = $self->{divestment};
  my $string;
  foreach my $acquisition (@​{$self->{acquisitions}}) {
  $string .= $acquisition->lineFormatString();
  }
  $string .= $self->divestmentLineFormatString . '-'x94 . "\n" .
  sprintf($summaryPattern, $divestment->symbol(), $self->ROI() || 0, (0 - $self->{costBasis}) || 0, $self->{revenue} || 0, $self->{realized} || 0); # <-- line 173
  return $string;
}
#####

This bug has not yet been reported to https://rt.cpan.org/Dist/Display.html?Name=Finance-StockAccount.

Thank you very much.
Jim Keenan

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @jkeenan

On Mon, 26 Jun 2017 20​:04​:36 GMT, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

Kudos to Slaven who digged it up!

bisect
------
commit 34f8e9c
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed May 31 11​:59​:48 2017 +0100

Perl_sv_vcatpvfn_flags​: simplify float_need calc

diagnostics
-----------
t/realization.t ............ 1/? panic​: snprintf buffer overflow at
/tmp/loop_over_bdir-114
42-Bx0l84/Finance-StockAccount-0.01-
0/blib/lib/Finance/StockAccount/Realization.pm line 173
.
# Tests were run but no plan was declared and done_testing() was not
seen.
# Looks like your test exited with 255 just after 16.
t/realization.t ............ Dubious, test returned 255 (wstat 65280,
0xff00)
All 16 subtests passed
[...]
t/allactivityregression.t (Wstat​: 65280 Tests​: 19 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
t/realization.t (Wstat​: 65280 Tests​: 16 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
t/set.t (Wstat​: 65280 Tests​: 22 Failed​: 0)
Non-zero exit status​: 255
Parse errors​: No plan found in TAP output
Files=13, Tests=201, 4 wallclock secs ( 0.04 usr 0.12 sys + 0.94
cusr 0.20 csys = 1.30
CPU)
Result​: FAIL
Failed 3/13 test programs. 0/201 subtests failed.

perl -V
-------
Summary of my perl5 (revision 5 version 27 subversion 1)
configuration​:
Commit id​: 34f8e9c
Platform​:
osname=linux
osvers=4.9.0-2-amd64
archname=x86_64-linux-thread-multi-ld
uname='linux k93msid 4.9.0-2-amd64 #1 smp debian 4.9.18-1 (2017-
03-30) x86_64 gnulinux '
config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-
perls/host/k93msid/v5.27.0-228-g34f8e9c776/5ea4 -Dmyhostname=k93msid
-Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db
-Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m
crypt sec util c cposix posix ucb BSD gdbm_compat -Duseithreads
-Duselongdouble -DDEBUGGING=-g'
hint=recommended
useposix=true
d_sigaction=define
useithreads=define
usemultiplicity=define
use64bitint=define
use64bitall=define
uselongdouble=define
usemymalloc=n
default_inc_excludes_dot=define
bincompat5005=undef
Compiler​:
cc='cc'
ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing
-pipe -fstack-protector-strong -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
optimize='-O2 -g'
cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing
-pipe -fstack-protector-strong -I/usr/local/include'
ccversion=''
gccversion='6.3.0 20170406'
gccosandvers=''
intsize=4
longsize=8
ptrsize=8
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=16
longdblkind=3
ivtype='long'
ivsize=8
nvtype='long double'
nvsize=16
Off_t='off_t'
lseeksize=8
alignbytes=16
prototype=define
Linker and Libraries​:
ld='cc'
ldflags =' -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/6/include-
fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu
/lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
libc=libc-2.24.so
so=so
useshrplib=false
libperl=libperl.a
gnulibc_version='2.24'
Dynamic Linking​:
dlsrc=dl_dlopen.xs
dlext=so
d_dlsymun=undef
ccdlflags='-Wl,-E'
cccdlflags='-fPIC'
lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-
strong'

Characteristics of this binary (from libperl)​:
Compile-time options​:
HAS_TIMES
MULTIPLICITY
PERLIO_LAYERS
PERL_COPY_ON_WRITE
PERL_DONT_CREATE_GVSV
PERL_IMPLICIT_CONTEXT
PERL_MALLOC_WRAP
PERL_OP_PARENT
PERL_PRESERVE_IVUV
PERL_USE_DEVEL
USE_64_BIT_ALL
USE_64_BIT_INT
USE_ITHREADS
USE_LARGE_FILES
USE_LOCALE
USE_LOCALE_COLLATE
USE_LOCALE_CTYPE
USE_LOCALE_NUMERIC
USE_LOCALE_TIME
USE_LONG_DOUBLE
USE_PERLIO
USE_PERL_ATOF
USE_REENTRANT_API
Built under linux
Compiled at Jun 26 2017 19​:29​:40
@​INC​:
/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-
228-g34f8e9c776/5ea4/lib/site_perl/5.27.1/x86_64-linux-thread-multi-ld
/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-
228-g34f8e9c776/5ea4/lib/site_perl/5.27.1
/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-
228-g34f8e9c776/5ea4/lib/5.27.1/x86_64-linux-thread-multi-ld
/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.0-
228-g34f8e9c776/5ea4/lib/5.27.1

This can be reduced to the test case attached. Run it against blead. The panic is associated with the addition of the 5th item in the printf formatting string​: %53.2f

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @jkeenan

131659-sprintf-format.pl

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @jkeenan

On Mon, 26 Jun 2017 22​:34​:33 GMT, jkeenan wrote​:

This can be reduced to the test case attached. Run it against blead.
The panic is associated with the addition of the 5th item in the
printf formatting string​: %53.2f

Thank you very much.

Or more simply to the second attachment.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

@p5pRT
Copy link
Author

p5pRT commented Jun 27, 2017

From @iabyn

On Mon, Jun 26, 2017 at 03​:42​:59PM -0700, James E Keenan via RT wrote​:

Or more simply to the second attachment.

my $zrealized = 69.2105000000001;
my $summaryPattern = "%53.2f\n";
printf($summaryPattern => ($zrealized));

Thanks for the reduction. I've just pushed this, which should fix it​:

commit 571ee10
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Tue Jun 27 09​:59​:41 2017 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue Jun 27 09​:59​:41 2017 +0100

  PERL_SNPRINTF_CHECK()​: off by 1 error
 
  PERL_SNPRINTF_CHECK() is used as part of a wrapper for snprintf()
  to check that snprintf didn't return more bytes than the buffer size
  given it to it (which should never happen anyway). But it was checking
  return_value >= buf_size rather than >. So a spurious panic could ensue
  if the formatted string exactly matched the buffer size.
 
  This hadn't been detected before because the old Perl_sv_vcatpvfn_flags()
  implementation added lots of fudge factors to the buffer size.
 
  At the same time, change the code in Perl_sv_vcatpvfn_flags() which grows
  PL_efloatbuf if its not big enough for float_need​:
 
  1) Make it require the buf size to be at least float_need + 1 rather than
  just float_need, to accommodate the \0 appended by snprintf() (we don't
  strictly need the \0, and a conforming snprintf() implementation should
  just return the string without trailing \0 if there isn't room for it,
  but its possible an snprintf() out there might stumble).
 
  2) When growing PL_efloatbuf, grow by an extra margin of 0x20, to
  reduce the likelihood of multiple reallocs.

--
Standards (n). Battle insignia or tribal totems.

@p5pRT
Copy link
Author

p5pRT commented Jun 27, 2017

From @jkeenan

On Tue, 27 Jun 2017 09​:09​:56 GMT, davem wrote​:

On Mon, Jun 26, 2017 at 03​:42​:59PM -0700, James E Keenan via RT wrote​:

Or more simply to the second attachment.

my $zrealized = 69.2105000000001;
my $summaryPattern = "%53.2f\n";
printf($summaryPattern => ($zrealized));

Thanks for the reduction. I've just pushed this, which should fix it​:

Confirmed.

#####
$ ./bin/perl -v | head -2 | tail -1
This is perl 5, version 27, subversion 2 (v5.27.2 (v5.27.1-77-g571ee10)) built for x86_64-linux

$ ./bin/cpanm Finance​::StockAccount
--> Working on Finance​::StockAccount
...
Building and testing Finance-StockAccount-0.01 ... OK
Successfully installed Finance-StockAccount-0.01
13 distributions installed
#####

I'll take this ticket for the purpose of closing it within 7 days unless Slaven or Andreas indicates otherwise.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jul 2, 2017

From @mauke

Am 27.06.2017 um 11​:09 schrieb Dave Mitchell​:

On Mon, Jun 26, 2017 at 03​:42​:59PM -0700, James E Keenan via RT wrote​:

Or more simply to the second attachment.

my $zrealized = 69.2105000000001;
my $summaryPattern = "%53.2f\n";
printf($summaryPattern => ($zrealized));

Thanks for the reduction. I've just pushed this, which should fix it​:

commit 571ee10

 1\) Make it require the buf size to be at least float\_need \+ 1 rather than
 just float\_need\, to accommodate the \\0 appended by snprintf\(\) \(we don't
 strictly need the \\0\, and a conforming snprintf\(\) implementation should
 just return the string without trailing \\0 if there isn't room for it\,
 but its possible an snprintf\(\) out there might stumble\)\.

I haven't looked at the code but this part of the commit message looks
wrong to me. AFAIK snprintf always writes a \0 (unless buf size == 0),
i.e. with

  char buf[3];
  int ret = snprintf(buf, 3, "%d", 9876);

I'd expect buf = { '9', '8', '\0' } (truncated) and ret = 4 (because ret
+ 1 is the buffer size snprintf would have needed).

In other words, it sounds like adding 1 is actually a hard requirement
to ensure correctness.

--
Lukas Mai <plokinom@​gmail.com>

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2017

From @iabyn

On Sun, Jul 02, 2017 at 10​:37​:07AM +0200, Lukas Mai wrote​:

Am 27.06.2017 um 11​:09 schrieb Dave Mitchell​:

On Mon, Jun 26, 2017 at 03​:42​:59PM -0700, James E Keenan via RT wrote​:

Or more simply to the second attachment.

my $zrealized = 69.2105000000001;
my $summaryPattern = "%53.2f\n";
printf($summaryPattern => ($zrealized));

Thanks for the reduction. I've just pushed this, which should fix it​:

commit 571ee10

 1\) Make it require the buf size to be at least float\_need \+ 1 rather than
 just float\_need\, to accommodate the \\0 appended by snprintf\(\) \(we don't
 strictly need the \\0\, and a conforming snprintf\(\) implementation should
 just return the string without trailing \\0 if there isn't room for it\,
 but its possible an snprintf\(\) out there might stumble\)\.

I haven't looked at the code but this part of the commit message looks wrong
to me. AFAIK snprintf always writes a \0 (unless buf size == 0), i.e. with

char buf\[3\];
int ret = snprintf\(buf\, 3\, "%d"\, 9876\);

I'd expect buf = { '9', '8', '\0' } (truncated) and ret = 4 (because ret + 1
is the buffer size snprintf would have needed).

In other words, it sounds like adding 1 is actually a hard requirement to
ensure correctness.

Ah so it is. So my commit was correct, but for the wrong reasons (and the
commit message was wrong).

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
  (with apologies to William Blake)

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2017

From @jkeenan

On Mon, 03 Jul 2017 08​:11​:10 GMT, davem wrote​:

On Sun, Jul 02, 2017 at 10​:37​:07AM +0200, Lukas Mai wrote​:

Am 27.06.2017 um 11​:09 schrieb Dave Mitchell​:

On Mon, Jun 26, 2017 at 03​:42​:59PM -0700, James E Keenan via RT
wrote​:

Or more simply to the second attachment.

my $zrealized = 69.2105000000001;
my $summaryPattern = "%53.2f\n";
printf($summaryPattern => ($zrealized));

Thanks for the reduction. I've just pushed this, which should fix
it​:

commit 571ee10

1) Make it require the buf size to be at least float_need + 1
rather than
just float_need, to accommodate the \0 appended by snprintf() (we
don't
strictly need the \0, and a conforming snprintf() implementation
should
just return the string without trailing \0 if there isn't room for
it,
but its possible an snprintf() out there might stumble).

I haven't looked at the code but this part of the commit message
looks wrong
to me. AFAIK snprintf always writes a \0 (unless buf size == 0), i.e.
with

char buf[3];
int ret = snprintf(buf, 3, "%d", 9876);

I'd expect buf = { '9', '8', '\0' } (truncated) and ret = 4 (because
ret + 1
is the buffer size snprintf would have needed).

In other words, it sounds like adding 1 is actually a hard
requirement to
ensure correctness.

Ah so it is. So my commit was correct, but for the wrong reasons (and
the
commit message was wrong).

Since the commit itself was correct, I'm marking the ticket Resolved as per schedule. Perhaps Dave could make another (whitespace?) commit which provides a better commit message.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jul 4, 2017

@jkeenan - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant