search

ssimms / pdfapi2

Create, modify, and examine PDF files in Perl
Other
15 stars 20 forks source link

fix uninitialized value in vec #25

Closed doojonio closed 4 years ago

doojonio commented 4 years ago

Hello. I've got few warnings like this:

-e: Use of uninitialized value in vec at .../PDF/API2/Resource/XObject/Image/PNG.pm line 250.
-e: Use of uninitialized value in scalar assignment at .../PDF/API2/Resource/XObject/Image/PNG.pm line 250.
-e: Use of uninitialized value in vec at .../PDF/API2/Resource/XObject/Image/PNG.pm line 251.
-e: Use of uninitialized value in scalar assignment at .../PDF/API2/Resource/XObject/Image/PNG.pm line 251.

Can this commit fix it?

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 56.824% when pulling f8365e07a41e122c6049fbaf662f44f845f6cb28 on toshikFedotov:uninitialized-in-vec into f9a972e2c7aa8b3ccc7f73e589299af841409c8a on ssimms:master.

PhilterPaper commented 4 years ago

Toshik, this is very odd, as this code has been running forever with no problems. What version of PDF::API2 are you running (2.037 is current), and what Perl version? I'm wondering if a very old or very new Perl version is failing to create and initialize $self->{' stream'} when written to by vec(), or is it $dict->{' stream'} that's giving the error? Or both? Are you running with non-standard Perl flags (-e?) that have just been silently passing this error all along without those flags? Could it be the source string $clearstream that's the problem, and not the target ' stream'? I take it you have run with your proposed change and no errors (and no change of behavior)?

doojonio commented 4 years ago

Sorry, but i don't know, what caused an error. This warnings are from logs from running server, so i don't know how to make same error call. It also could be unitialized $clearstream, i just made a hypothesis and proposed the solution. The Perl version is 5.16.3 and the module versions is 2.033

PhilterPaper commented 4 years ago

2.033 is a little old, but I don't think any of the PNG code has changed since then. Making the changes you showed in this PR, and no other changes, got rid of the error messages? If so, it's probably not $clearstream to blame. Your server might have some extra flags set (such as -e) that most people don't normally run with, and it flagged the missing ' stream' element creation.

I don't know enough about the internals of Perl (garbage collection) to say whether setting ' stream' to an empty string is any better or worse than deleting ' stream' and then initializing it to an empty string.

PhilterPaper commented 4 years ago

If you run perl -e testprogram.pl without your patch, does it show the error, and with the patch, no error? testprogram would just be something to display a PNG image on a page. I tried your patch in PDF::Builder, but (even though it appears to run correctly) I'm now a little concerned about whether you actually have tested this patch to see if it gets rid of the error (and doesn't introduce any new ones). You should not propose a PR unless you have tested the code and found that it does the job. It is not appropriate to issue a PR with a speculative fix.

doojonio commented 4 years ago

Okay, let me try to find cause of this error, write fix and few new tests. I'll comeback soon