ext/standard/iptc.c: drop some unused return values #16312
Conversation
| @@ -73,14 +73,12 @@ | |||
| #define M_APP15 0xef | |||
|
|
|||
| /* {{{ php_iptc_put1 */ | |||
| static int php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf) | |||
| static void php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf) | |||
| { | |||
| if (spool > 0) | |||
| PUTC(c); | |||
There was a problem hiding this comment.
Actually, I think we should check the return value of PUTC(); unless that is 1, something is wrong, and we should return from php_iptc_put1(), and so on.
"If a function be advertised to return an error code in the event of difficulties, thou shalt check for that code, yea, even though the checks triple the size of thy code and produce aches in thy typing fingers, for if thou thinkest it cannot happen to me, the gods shall surely punish thee for thy arrogance." – Henry Spencer
There was a problem hiding this comment.
Okay, but I have no idea what this code is meant to be doing - should I just revert the changes to php_iptc_put1?
There was a problem hiding this comment.
Before voidifying anything in this file, I would suggest to find out how that spool is supposed to work. The documentation states:
If the spool flag is less than 2 then the JPEG will be returned as a string. Otherwise the JPEG will be printed to STDOUT.
That seems to be somewhat true, but only if spool>0 actually something is output. Somewhat confusing (might have historic reasons), and might be simplified. But at least the documentation should be improved.
Then I would also suggest to check what's up with the fp parameter to php_iptc_put1() which is apparently unused, so this is unnecessarily confusing; on a quick glance, one might actually assume that the JPEG file is overwritten in place, and the documentation supports this assumption. But apparently the parameter is unused since porting from PHP 3, which happened 25 years ago.
No description provided.