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
