Skip to content

ext/standard/iptc.c: drop some unused return values #16312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ext/standard/iptc.c: drop some unused return values #16312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions ext/standard/iptc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but I have no idea what this code is meant to be doing - should I just revert the changes to php_iptc_put1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if (spoolbuf) *(*spoolbuf)++ = c;

return c;
}
/* }}} */

Expand All @@ -106,32 +104,28 @@ static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf)
/* }}} */

/* {{{ php_iptc_read_remaining */
static int php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf)
static void php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf)
{
while (php_iptc_get1(fp, spool, spoolbuf) != EOF) continue;

return M_EOI;
}
/* }}} */

/* {{{ php_iptc_skip_variable */
static int php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf)
static void php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf)
{
unsigned int length;
int c1, c2;

if ((c1 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
if ((c1 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return;

if ((c2 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
if ((c2 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return;

length = (((unsigned char) c1) << 8) + ((unsigned char) c2);

length -= 2;

while (length--)
if (php_iptc_get1(fp, spool, spoolbuf) == EOF) return M_EOI;

return 0;
if (php_iptc_get1(fp, spool, spoolbuf) == EOF) return;
}
/* }}} */

Expand Down
Loading