minor issues (found by cppcheck)

minor issues (found by cppcheck)

[Szabolcs Nagy]

i fixed some minor issues in my repo which were found by cppcheck
here is a list of the non-fixed ones:


[src/misc/mntent.c:32]: (portability) scanf without field width limits can crash with huge input data

getmntent_r has a sscanf with %d,
it might make sense to limit the width


[src/regex/regcomp.c:2032]: (performance) Variable 'status' is reassigned a value before the old one has been used.
[src/regex/regcomp.c:3133]: (warning) Redundant assignment of 'errcode' to itself.
[src/regex/regcomp.c:2060]: (style) Variable 'minimal_tag' is assigned a value that is never used.
[src/regex/regcomp.c:108]: (style) struct or union member 'Anonymous1::params' is never used.
[src/regex/regcomp.c:2803]: (error) Uninitialized variable: params

some of these occure multiple times,
the last two is probably worth fixing:
u.params in tre_literal_t struct is never used
and in the tre_match_empty function the params
argument is never used, but an uninitialized
pointer is passed anyway


[src/locale/strfmon.c:33]: (style) Variable 'fill' is assigned a value that is never used.
[src/stdio/vfscanf.c:134]: (style) Variable 'alloc' is assigned a value that is never used.
[src/stdio/vfwscanf.c:144]: (style) Variable 'alloc' is assigned a value that is never used.

these are examples of unused values but they
all seem to be innocent

Re: minor issues (found by cppcheck)

[Rich Felker]

On Mon, Jan 14, 2013 at 08:41:47PM +0100, Szabolcs Nagy wrote:
> i fixed some minor issues in my repo which were found by cppcheck
> here is a list of the non-fixed ones:
> 
> 
> [src/misc/mntent.c:32]: (portability) scanf without field width
> limits can crash with huge input data

This looks bogus. %s and %[ are used only with the * modifier which
inhibits storage.

> getmntent_r has a sscanf with %d,
> it might make sense to limit the width

I think the error is irrelevant for %d unless we're talking about the
theoretical UB for integer overflow, but that doesn't seem to be what
this warning is about. Anyway, musl's scanf has well-defined overflow
behavior.

> [src/regex/regcomp.c:2032]: (performance) Variable 'status' is reassigned a value before the old one has been used.
> [src/regex/regcomp.c:3133]: (warning) Redundant assignment of 'errcode' to itself.
> [src/regex/regcomp.c:2060]: (style) Variable 'minimal_tag' is assigned a value that is never used.
> [src/regex/regcomp.c:108]: (style) struct or union member 'Anonymous1::params' is never used.
> [src/regex/regcomp.c:2803]: (error) Uninitialized variable: params
> 
> some of these occure multiple times,
> the last two is probably worth fixing:
> u.params in tre_literal_t struct is never used
> and in the tre_match_empty function the params
> argument is never used, but an uninitialized
> pointer is passed anyway

Indeed, these should be fixed.

> [src/locale/strfmon.c:33]: (style) Variable 'fill' is assigned a value that is never used.
> [src/stdio/vfscanf.c:134]: (style) Variable 'alloc' is assigned a value that is never used.
> [src/stdio/vfwscanf.c:144]: (style) Variable 'alloc' is assigned a value that is never used.
> 
> these are examples of unused values but they
> all seem to be innocent

They're all cases of unimplemented features, so the code should not be
removed, but rather finished. :-)

Rich

Re: minor issues (found by cppcheck)

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2013-01-14 16:05:41 -0500]:
> On Mon, Jan 14, 2013 at 08:41:47PM +0100, Szabolcs Nagy wrote:
> > getmntent_r has a sscanf with %d,
> > it might make sense to limit the width
> 
> I think the error is irrelevant for %d unless we're talking about the
> theoretical UB for integer overflow, but that doesn't seem to be what
> this warning is about. Anyway, musl's scanf has well-defined overflow
> behavior.

i mean if there is a very long digit sequence in the input
it may make sense to fail early, eg use %11d and with a final
%n we could check if the input is read correctly

assuming we want to report failure on invalid input

Re: minor issues (found by cppcheck)

[Rich Felker]

On Mon, Jan 14, 2013 at 10:23:50PM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@aerifal.cx> [2013-01-14 16:05:41 -0500]:
> > On Mon, Jan 14, 2013 at 08:41:47PM +0100, Szabolcs Nagy wrote:
> > > getmntent_r has a sscanf with %d,
> > > it might make sense to limit the width
> > 
> > I think the error is irrelevant for %d unless we're talking about the
> > theoretical UB for integer overflow, but that doesn't seem to be what
> > this warning is about. Anyway, musl's scanf has well-defined overflow
> > behavior.
> 
> i mean if there is a very long digit sequence in the input
> it may make sense to fail early, eg use %11d and with a final
> %n we could check if the input is read correctly
> 
> assuming we want to report failure on invalid input

Is a line containing a number 1 represented as a billion zeros
followed by a 1 "invalid input"? It's pathological, but I don't see
any reason to go out of our way to treat it as invalid, especially
since the files being parsed by this function are provided either by
root or by the invoking user (usually by root).

Rich