On Mon, 2019-01-07 at 12:41 -0800, Dan Williams wrote:
> On Mon, Jan 7, 2019 at 11:03 AM Verma, Vishal L
> <[email protected]> wrote:
> > 
> > 
> > On Mon, 2019-01-07 at 10:56 -0800, Dan Williams wrote:
> > > On Mon, Jan 7, 2019 at 10:49 AM Verma, Vishal L
> > > <[email protected]> wrote:
> > > > 
> > > > 
> > > > On Mon, 2019-01-07 at 17:38 +0900, QI Fuli wrote:
> > > > > Import ciniparser tool from ccan [1], therefore, the ndctl
> > > > > monitor can
> > > > > read the configuration file by using this library.
> > > > > 
> > > > > [1]: https://ccodearchive.net/info/ciniparser.html
> > > > > 
> > > > > Signed-off-by: QI Fuli <[email protected]>
> > > > > ---
> > > > >  Makefile.am                  |   6 +-
> > > > >  ccan/ciniparser/LICENSE      |   1 +
> > > > >  ccan/ciniparser/ciniparser.c | 480
> > > > > +++++++++++++++++++++++++++++++++++
> > > > >  ccan/ciniparser/ciniparser.h | 262 +++++++++++++++++++
> > > > >  ccan/ciniparser/dictionary.c | 266 +++++++++++++++++++
> > > > >  ccan/ciniparser/dictionary.h | 166 ++++++++++++
> > > > >  6 files changed, 1180 insertions(+), 1 deletion(-)
> > > > >  create mode 120000 ccan/ciniparser/LICENSE
> > > > >  create mode 100644 ccan/ciniparser/ciniparser.c
> > > > >  create mode 100644 ccan/ciniparser/ciniparser.h
> > > > >  create mode 100644 ccan/ciniparser/dictionary.c
> > > > >  create mode 100644 ccan/ciniparser/dictionary.h
> > > > 
> > > > Hi Qi,
> > > > 
> > > > Thanks for these patches, and also for rebasing to the latest!
> > > > 
> > > > ciniparser.c adds a new warning (see below). Could you fix that up
> > > > in a
> > > > new patch on top of the initial import (i.e. we retain the as-is
> > > > import, and make a record of what we changed).
> > > > 
> > > > Looks like the -Wformat-truncation= warnings were first introduced
> > > > in
> > > > gcc-7.1, but only became really zealous after gcc-8.1. The right
> > > > solution here might just be to suppress it by adding a -Wno-format-
> > > > truncation to CFLAGS, but I'm open to considering other
> > > > alternatives.
> > > 
> > > That warning looks pretty handy.
> > > 
> > > > 
> > > >    $ gcc --version
> > > >    gcc (GCC) 8.2.1 20181105 (Red Hat 8.2.1-5)
> > > > 
> > > >    ccan/ciniparser/ciniparser.c: In function ‘ciniparser_load’:
> > > >    ccan/ciniparser/ciniparser.c:442:39: warning: ‘%s’ directive
> > > > output may be truncated writing up to 1024 bytes into a region of
> > > > size between 0 and 1024 [-Wformat-truncation=]
> > > >        snprintf(tmp, ASCIILINESZ + 1, "%s:%s", section, key);
> > > >                                           ^~            ~~~
> > > >    In file included from /usr/include/stdio.h:873,
> > > >                     from ./ccan/ciniparser/ciniparser.h:39,
> > > >                     from ccan/ciniparser/ciniparser.c:36:
> > > >    /usr/include/bits/stdio2.h:67:10: note:
> > > > ‘__builtin___snprintf_chk’ output between 2 and 2050 bytes into a
> > > > destination of size 1025
> > > >       return __builtin___snprintf_chk (__s, __n,
> > > > __USE_FORTIFY_LEVEL - 1,
> > > >              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~~~~~
> > > >            __bos (__s), __fmt, __va_arg_pack ());
> > > >            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > Since it's an snprintf without the error return being checked,
> > > perhaps
> > > it would be happier as an sprintf with a precision specified to limit
> > > the output?
> > 
> > That would work, but 'section' and 'key' are both ASCIILINESZ+1, so how
> > would we specify precision for both the %s specifiers? i.e. would we
> > have to make up an artificial split?
> 
> Good point. No way to do it without variable precision. However,
> looking closer I think the warning only triggers when the return value
> is not checked, so perhaps better to just add error handling to that
> case.

Yep that seems reasonable to me.

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to