Re: [pdf-devel] Task FS#110

[Gustavo Martin Domato]

Hi Aleksander,

> Hi Gustavo,
> 
> > I've been looking at the task and I found that, even for those problems
> > regarding null-dereferencing or uninitialized variables using, most
> > aren't problems at all.
> > 
> 
> That happens a lot yes :-/
> 
> > For example:
> > 
> > utils/pdf-reader.c, stated as "Uninitialized return value" doesn't have
> > a problem, since variable *read_mode needs to be true and false at the
> > same time in order to make that happen.
> > 
> 
> You mean utils/pdf-filter.c, right?
> 
> It's a pretty weird thing that Clang didn' catch that, interesting.
> Anyway, I just initialized the 'stm' variable to NULL and that should
> fix it. The change is already in trunk:
> http://bzr.savannah.gnu.org/lh/pdf/libgnupdf/trunk/revision/935
> 

Other (sometimes) errors are like that one. At files:

torture/unit/base/text/pdf-text-set-unicode.c, at fun
test_one_string(...) -> Initialize variable input_data to NULL should
do.
torture/unit/base/text/pdf-text-set-from-unicode.c, at same function ->
Same (seems the code was copied).

src/base/pdf-text.c at lines 1802/1804 ... well, that Clang thing
doesn't seem to realize you can't continue if an assertion is false. I
can't figure out how to correct that.

lib/getopt.c line 852 - This one is disturbing to me. It shouldn't be
possible because of the function's preconditions (longopt must be an
array with a null name at the end), but in the code it appears to be
that longopt can effectively be null (tested at line 476) and p=longopt
is actually derreferenced in the loop control statement at line 852... I
trust the gnulib better that me, but...

> > Is there a way to report this, so that future contributors don't need to
> > spend time trying to figure it out again?
> 
> That is a good question. I've used other static analysis tools that
> allow marking issues to be ignored through a web-based interface, but
> it's not the case here, reports are static HTML. Maybe there's a way of
> generating a file with things to ignore by clang? Something like
> valgrind's suppressions file... would be good to check that.
> 

I would check... if I knew how to :P

> > 
> > Also send a tiny patch to a tiny problem that seemed real to me... 
> > 
> 
> Thanks for that patch, it really is a bug being fixed (using a variable
> without initializing it). But I don't think we should apply it right
> now, as I already refactored pdf-filereader in a not-yet-merged branch,
> also removing that issue. See:
> http://bazaar.launchpad.net/~aleksander-m/gnupdf/fsys-api/view/head:/utils/pdf-filereader.c#L540
> That branch should hit trunk soon.
> 
> Thanks for the patch anyway! :-)

You're welcome.

> 
> Cheers,
> 


Cheers,
Gustavo