Need to allow for the possibility that skinr_skin_load() will return NULL when called from skinr_ui_admin_skins()

One-line patch attached.

CommentFileSizeAuthor
#4 995794-skinr-skin-load-null.patch2.75 KBcoltrane
skinr-HEAD.patch560 bytespillarsdotnet

Comments

jacine’s picture

jacine
she/her
Location New York
commented
Priority: Minor » Normal

This looks good to me. Would be good if someone else could take a quick look.

This patch fixes the remaining Simpletest issues causing it to fail right now.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work 
+++ skinr_ui.admin.inc	10 Dec 2010 20:50:45 -0000
@@ -255,6 +255,7 @@
+      if (empty($skins)) continue;

Would be beneficial to prepend a comment to this condition in order to explain the edge-case of skinr_ui_admin_skins().

Wrong control structure syntax, see http://drupal.org/coding-standards

Powered by Dreditor.

jacine’s picture

jacine
she/her
Location New York
commented

Would be beneficial to prepend a comment to this condition in order to explain the edge-case of skinr_ui_admin_skins().

Thanks @sun. I'm not sure what the edge-case is, so yeah, a comment would be nice.

coltrane’s picture

coltrane
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new 995794-skinr-skin-load-null.patch2.75 KB

How about checking if $skins is !empty() before looping?

jacine’s picture

jacine
she/her
Location New York
commented
Status: Needs review » Fixed

Or that... :)

Committed. Thanks! http://drupal.org/cvs?commit=466096

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.