file_munge_filename() doc mentions watchdog but shouldn't

Agreed. watchdog() is not being called, and should be removed from the doc.

Please fix in Drupal 7 first, then backport patch to Drupal 6.

Here's some better (I think) doc for file_munge_filename().

Yes, patch applies on head.

Comment changes only. Seems thorough.

Watchdog was removed. Ship it!

Yeah! First patch review!

Overall, a pass from me. Applies fine, solves the original problem by removing reference to watchdog(), and clarifies the rest of the documentation too. I might clarify the line which reads

If variable 'allow_insecure_uploads' evaluates to TRUE, no alterations will be made.

As it isn't immediately clear what this variable is, where it is set, etc.; but I would still pass the patch even if this line was kept as it currently is.

Changing status to RTBC - accidentally changed away from this as we were both reviewing at the same time.

But FWIW, shouldn't patches only be set to RTBC when they've been received two positive reviews?

Committed to CVS HEAD. Thanks!

Needs port to Drupal 6.

Here's a patch for Drupal 6.

The comment doesn't address the obvious questions: When should filenames be munged? Why is “exploit.php.pps” a dangerous name? How do I choose the extensions that should not be altered?

Please file this as a separate issue on Drupal 7. The original issue report here is that the doc mentions watchdog but shouldn't. If you think the doc needs more clarification in general, that is a separate issue.

Thanks!

Done: #710640: Improve documentation for file_munge_filename()

Since this D6 patch sets the doc to exactly what it is in D7, and that was all approved and committed, I'm going to go ahead and set this to RTBC.

Thanks, committed to Drupal 6.