[wp-trac] [WordPress Trac] #14928: wp_mkdir_p() phpdocs do not explain the "p" in the function name in full

[WordPress Trac]

#14928: wp_mkdir_p() phpdocs do not explain the "p" in the function name in full
--------------------------+-------------------------------------------------
 Reporter:  hakre         |       Owner:                 
     Type:  defect (bug)  |      Status:  new            
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Inline Docs   |     Version:  3.0            
 Severity:  normal        |    Keywords:  needs-patch    
--------------------------+-------------------------------------------------

Comment(by hakre):

 I now provided a patch. It updates the PHPDOC to make the functionality
 more clear. The "p" in "wp_mkdir_p()" stands for parent directory (not
 permission).

 While checking the functionality of the function to make the docblock more
 clear I reviewed the code as well. I re-factored the function on the way.
 Here is my report:

 '''Umask Implications (Informative)'''

 I checked it against implications discussed in #10170 / [11667] (default
 directory permissions mode). Umask is handeled because chmod() is used
 after directory creation.

 '''0 Permission Edge-Cases (Improvement)'''

 The function could not properly handle permissions for newly created
 directories with no parent directory and/or when the acquisition of the
 parents status failed. This did result in chmod $target to 0 for those
 edge cases.

 I fixed this by having a fall-back to 0755 / FS_CHMOD_DIR (if defined) as
 we do in class WP_Filesystem_Direct.

 '''Recursion Order (Improvement)'''

 Next to that, the recursion of the function could be improved to take care
 on the parent prior to create the concrete directory. Currently per each
 directory name in the full path it is trying to create it first before
 taking care of the parent. This will always fail if the parent directory
 does not exists.

 I fixed this by reversing the order. Prior to creation of the directory,
 it's taken care that the parent is handled first if the target has a
 parent. This is a major improvement and makes the function less complex.

 '''Unknown/Undocumented Normlization (Informative)'''

 Then there is some php.net user contributed note referenced I could not
 find. I marked that part as undocumented.  But I left the functionality
 unchanged. I do not properly understand the motivation of str_replace()
 operation fully. It is unable to resolve multitudes (e.g. /// or ////)
 anyway. Maybe someone can shed some light in there, I probably might just
 miss something here.

 '''Missing Writeable Check (Informative)'''

 The function did not (and does not in my patch) check whether or not the
 directory is writable at the end of the call (see related [comment:9
 comment by Denis]). It gets chmod'ed to the parent folder's permissions
 which probably implicates that (as the new folder can be created, the
 parent folder's permission must allow to so). But it was and is not
 checked whether that chmod operation is successful.

 In the current code a newly created folder might have been set to 0
 successfully which is not writeable in some edge-cases.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/14928#comment:11>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software