incubator-mod_ftp-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Querna <c...@force-elite.com>
Subject Re: mod_ftp patch: New directive FTPDirUmask
Date Tue, 28 Mar 2006 06:21:16 GMT
Kanagasabai Sriskanthaverl wrote:
> Hi guys,
> I have written a patch for mod_ftp, which defines a new directive
> FTPDirUmask, allows to configure the umask for MKDIR command.


I like the idea.  I think we should add this.

...snip...
> ===================================================================
> --- src/ftp_commands.c	(revision 388162)
> +++ src/ftp_commands.c	(working copy)
....
> @@ -856,6 +859,9 @@
>      else {
>          fc->response_notes = apr_psprintf(r->pool, FTP_MSG_DIR_CREAT,
>                                            r->parsed_uri.path);
> +#ifdef HAVE_FCHMOD
> +	chmod(r->filename, ftp_unix_perms2mode(fsc->dirperms));
> +#endif
>          return FTP_REPLY_PATH_CREATED;
>      }
>  }


Is there any reason this cannot use apr_file_perms_set instead of chmod?

I believe via the File Permissions Flags, you should be able to do 
everything, but with more portability:
http://apr.apache.org/docs/apr/group__apr__file__permissions.html

It would also be helpful to note in a comment, why using the permissions 
param to apr_dir_make() is not enough, since we don't want to set the 
process wide UMask....

> Index: src/mod_ftp.c
> ===================================================================
> --- src/mod_ftp.c	(revision 388162)
> +++ src/mod_ftp.c	(working copy)
.....
> @@ -813,6 +866,8 @@
>  		 "Run an FTP server on this host"),
>      AP_INIT_TAKE1("FTPUmask", ftp_umask, NULL, RSRC_CONF,
>  		  "Set the umask for created files"),
> +    AP_INIT_TAKE1("FTPDirUmask", ftp_dirumask, NULL, RSRC_CONF,
> +		  "Set the umask for created directory"),
>      AP_INIT_TAKE1("FTPTimeoutLogin", ftp_set_int_slot,
>                    (void *)APR_XtOffsetOf(ftp_server_config, 
>                                           timeout_login), RSRC_CONF, 

It seems like we might want to inherit the value for FTPDirUmask from 
the value of FTPUmask, if FTPDirUmask is not set? Thoughts?

Thanks,

-Paul


Mime
View raw message