- (array) $field['columns'] is unsafe and lazy code
- the $columns variable is not initialized

CommentFileSizeAuthor
#10 778249-storage-details.patch2.22 KBmr.baileys
#4 778240-storage-details.patch2.37 KBmr.baileys
#2 778240-field-columns.patch2.19 KBdawehner
#1 778240-field-columns.patch852 bytesdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new 778240-field-columns.patch852 bytes
dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 778240-field-columns.patch2.19 KB

This changed the code in the test, too

yched’s picture

yched commented
Status: Needs review » Needs work

field_read_fields() takes care of initializing $field['columns'] as an empty array, so there shouldn't be any need for duplicating this down the line. This patch can just remove the (array) cast.
Other change is good, of course.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
StatusFileSize
new 778240-storage-details.patch2.37 KB

Re-rolled with yched's comments from #3. Additionally removed $details = array(); from field_test_field_storage_details() and field_sql_storage_field_storage_details() as it is not used.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Reviewed & tested by the community

This is a nice cleanup.

grendzy’s picture

grendzy commented

#4: 778240-storage-details.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 778240-storage-details.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Needs to be re-rolled.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented

Marked #909642: Wrong array instantiated in field_sql_storage_field_storage_details() as duplicate.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
StatusFileSize
new 778249-storage-details.patch2.22 KB

Rerolled tokeep up with HEAD.

mdm’s picture

mdm commented

#10 works and passes testing for me.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.