if ($radius == 0) {
  return $img; imagedestroy($img);
  break;
}

php7 rightfully rejects this bit since the break is invalid in this context not to mention that code cannot be executed since it is after the return

CommentFileSizeAuthor
#3 invalid_syntax_logic-2639570-3.patch435 bytesjoelpittet
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

boombatower created an issue. See original summary.

boombatower’s picture

boombatower CreditAttribution: boombatower commented

simple solution was to drop this code, but present in 6.x as well

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Active » Needs review
FileSize
invalid_syntax_logic-2639570-3.patch435 bytes

Removing unexecuted code.

drclaw’s picture

drclaw CreditAttribution: drclaw commented

Patch looks good and applies cleanly. TBH I'm actually not 100% sure how to test it, but I'm pretty confident the removed code would never be run anyway since it's after the return statement so I'm happy to call this RTBC.

Does it matter that the function is pulled from http://vikjavev.no/computing/ump.php and that the extraneous code is in that source as well?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

Nice find, I didn't noticed that but at least the copyright is left in there. There doesn't seem to be a license attached to that bit of code and the author suggests copying it.

mike503’s picture

mike503 CreditAttribution: mike503 commented

+1 the fix is correct.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

@mike503, if the fix works in your testing please set the status to "Reviewed and Tested by the community"

mike503’s picture

mike503 CreditAttribution: mike503 commented
Status: Needs review » Reviewed & tested by the community
nickonom’s picture

nickonom CreditAttribution: nickonom commented

I also confirm the patch takes care of the issue. Ready to be committed.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody commented

Confirming RTBC. Can we commit this please? Also a new stable release would make sense then perhaps?

jghyde’s picture

jghyde CreditAttribution: jghyde commented

#3 works and tested. Commit it!

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia commented
Issue tags: +PHP 7.0 (duplicate)

Bump, ran into this in D6 as well, it's haunting me:)

SivaprasadC’s picture

SivaprasadC CreditAttribution: SivaprasadC as a volunteer and at DrupalPartners for Innoppl Technologies Pvt. Ltd commented

#3 works like a charm. Confirming RTBC.

GiorgosK’s picture

GiorgosK
Location Chios, Greece 🇬🇷
CreditAttribution: GiorgosK commented

#3 works RTBC.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
CreditAttribution: jwilson3 commented

RTBC++ #3 Applies cleanly against Drupal 6 imageapi.