The comment for FILE_STATUS_* flags says that it should be binary flags with the first 8 bits reserved. I can see that the first two are 0 for TEMPORARY and 1 for PERMANENT which might have been old status when it was a boolean instead of a binary flag. On line 1227 of the system.module within system_cron() there is a SQL query that is used to get all temporary files over a certain time to delete them. The SQL query is doing a straight status = %d when it should be doing a bitwise operator. Attached is a patch to do that. Unfortunately I am not sure if anything needs to be changed for the file_set_status() function within the file.inc. From my understanding that file_set_status() should be a bitwise OR instead of a straight up assignment. Reasoning for this is that the original comment for the FILE_STATUS says that contrib modules could create their own status. Well if two modules have different status flags and they don't know about each other then they should only really be setting the bits that they care about and leaving the other bits to whatever the other contrib modules have set for that file.
| Comment | File | Size | Author |
|---|---|---|---|
| drupal-6.9-core-system.module-FILE_STATUS-bitwise-select.patch | 1.02 KB | ShawnClark |
Comments
Comment #1
ShawnClark commentedIf the binary functionality is not suppose to be part of the FILE_STATUS then the comment should be updated for the flags so that other contrib modules don't try to use this.
Comment #2
dave reidThis is how it's supposed to be. It was fixed in 7.x but needs to be backported. Current system_cron 7.x:
Comment #3
dave reidComment #4
ShawnClark commentedQuestion ... should it be a check against the bit flag != FILE_STATUS_PERMANENT or == FILE_STATUS_TEMPORARY? The patch i have is the later but I can update it to be the same as the 7.x code.
Comment #5
ShawnClark commentedSorry for the double comment but just thought about one more thing on this. Shouldn't there really only be one bit flag for FILE_STATUS_PERMANENT as a bit is a boolean condition. Right now it could be that it is both or none of PERMANENT / TEMPORARY and I am guessing that is not a recommended situation?
Comment #6
ShawnClark commentedComment #7
dpearcefl commentedIs this still an active issue?
Comment #8
jkingsnorth commentedNo response for a long time, so closing this issue. Please reopen if it is still a problem.