[sane-devel] unpaper default parameter - empty output bug

[Jens Gulden]

Bertrik Sikken wrote:
 > I put my patch in the place mentioned above, but I guess you
 > probably already fixed most of the critical things.

Thanks for your help. In fact we did most of the things in parallel, but 
cross-validating our work this way can only increase quality... I also 
added the extra include-directive (stdlib), so I think the current 
version now fully includes all changes from your patch. In addition to 
that I made minor corrections to the documentation and fixed the 
"variable 'half' uninitialized"-bug (see below).

 > The stack usage issue is not fixed and I am not sure if it is
 > really too much or not (about half a megabyte I guess).

Wouldn't we get some error message in case of stack-overflows? I would 
expect some segmentation fault or other raised signal in that case. So I 
also haven't done anything about the stack yet.

 > In warnings.txt is a warning about a variable 'half' that might
 > not be initialized before use. I took a quick look at it and to
 > me it appears the compiler is right to complain about it.

Yes! That was another potential bug, although it seems it was not the 
reason for breaking the optimized version (I didn't have that fix in the 
version I posted saturday, and still this one seemed to work.)
It should be fixed now.

 > I haven't looked at the other possibly uninitialized variables.
These are ok, because their use is intentionally optional in the program 
flow.
I have also kept the yet useless parameter 'type' of getPixel() and 
setPixel() for possible future extensions (it should be optimized-away 
anyway - although I have made some bad experiences recently with my 
intuitive notion of what the optimizer does...:).

The remaining warnings with -pedantic and -Wall switched on are:

  [exec] unpaper.c:27: warning: string length `1303' is greater than the 
length `509' ISO C89 compilers are required to support
      [exec] unpaper.c:316: warning: string length `18802' is greater 
than the length `509' ISO C89 compilers are required to support
      [exec] unpaper.c:317:1: warning: C++ style comments are not 
allowed in ISO C90
      [exec] unpaper.c:317:1: warning: (this will be reported only once 
per input file)
      [exec] unpaper.c: In function `rotate':
      [exec] unpaper.c:1398: warning: implicit declaration of function 
`sincos'
      [exec] unpaper.c: In function `main':
      [exec] unpaper.c:2294: warning: `outputFilename' might be used 
uninitialized in this function
      [exec] unpaper.c:2300: warning: `qpixelBuffer' might be used 
uninitialized in this function
      [exec] unpaper.c:2302: warning: `inputTypeName' might be used 
uninitialized in this function
      [exec] unpaper.c:2315: warning: `startTime' might be used 
uninitialized in this function
      [exec] unpaper.c:2316: warning: `endTime' might be used 
uninitialized in this function
      [exec] unpaper.c: In function `setPixel':
      [exec] unpaper.c:634: warning: unused parameter `type'
      [exec] unpaper.c: In function `getPixel':
      [exec] unpaper.c:653: warning: unused parameter `type'

I will make an update version 1.0.1 out of the new code and will 
announce it to this list soon.

 > Kind regards,
 > Bertrik

Thanks and groetjes
Jens



Bertrik Sikken schrieb:
> Jens Gulden wrote:
> 
>> Bertrik Sikken wrote:
>>
>>> I'm working on a patch and I'll post it when I'm done at:
>>> http://home.kabelfoon.nl/~bertrik/unpaper/
>>
>>
>>
>> I cleaned up the code at some places, so much less warnings remain 
>> when compiling with -Wall -W -pedantic. (Thanks for the hints, 
>> Bertrik, Monty, Frank.)
>>
>> And in fact the problems at least on the one machine I found have 
>> disappeared, the -O3 optimized version runs there now.
>>
>> If, together with your patch, this turns out to be a more stable 
>> version, I will put an updated version of distribution archive on the 
>> project site and commit the changes in CVS.
>>
>> Jens
> 
> 
> I put my patch in the place mentioned above, but I guess you
> probably already fixed most of the critical things.
> 
> Basically the patch does this:
> * replaced 'const' by '#define'
> * removed unused variables (this is not strictly necessary)
> * added missing return value
> * fixed some printfs
> * added an include file
> 
> The stack usage issue is not fixed and I am not sure if it is
> really too much or not (about half a megabyte I guess).
> In warnings.txt is a warning about a variable 'half' that might
> not be initialized before use. I took a quick look at it and to
> me it appears the compiler is right to complain about it.
> I haven't looked at the other possibly uninitialized variables.
> 
> Kind regards,
> Bertrik