Attached patch does the following things:

1. Removes FILE_STATUS_TEMPORARY. Because $file->status is a bitmapped field, having FILE_STATUS_TEMPORARY for the 0 value of this field doesn't make sense.
2. Fixes issues with the use of FILE_STATUS_PERMANENT to ensure that it is treated more uniformly as a binary flag.
3. Fixes a logic issue with $file->status in upload.module.

At present all tests still pass.

CommentFileSizeAuthor
#1 file_353207.patch5.91 KBdrewish
remove-file_status_temp.patch5.74 KBtheflowimmemorial

Comments

drewish’s picture

drewish commented
Status: Active » Needs review
StatusFileSize
new file_353207.patch5.91 KB

Glad you took this up. The whole temporary constant has bothered me for a while but I'd never gotten around to rolling a patch.

I cleaned up the file.inc docs to make it clear what FILE_STATUS_PERMANENT does since FILE_STATUS_TEMPORARY's comment is being removed.

I fiexed the spacing on + $file->status |= FILE_STATUS_PERMANENT; to line up with all the other assignments.

I dropped the changes to system_cron() since the current code is fine.

dries’s picture

dries commented
Status: Needs review » Fixed

Good clean-up. Committed to CVS HEAD. Kudos to both.

drewish’s picture

drewish commented

thanks, added the change to the update docs: http://drupal.org/node/224333#remove_FILE_STATUS_TEMPORARY

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

I have no experience with bitmap flags (or whatever they're called), but removing FILE_STATUS_TEMPORARY seems very bad for readability from my point of view.

drewish’s picture

drewish commented

it's less readable but the problem with it was that it didn't really do anything and encouraged people to set the status in a way that would preclude other bits from being used.

Status: Fixed » Closed (fixed)

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

amontero’s picture

amontero
Location Barcelona
commented

Linking to related feature request for consideration:
#1659116: Define FILE_STATUS_TEMPORARY and expose $status in file_save_data