Code review comment for lp:~xnox/upstart/user-log-dir

On 22 January 2013 10:28, James Hunt <email address hidden> wrote:
>
> * init/xdg.c:
> - get_home_subdir(): Return NULL immediately if nih_sprintf() fails.
> - get_user_log_dir(): Return NULL immediately if nih_sprintf() fails.

Done.

> - create_dir():
> - for consistency, this should probably call
> init/system.c:int system_mkdir(const char *path, mode_t mode) passing
> @dir and 0700.
> - should assert @dir: I don't understand why it currently handles
> a NULL directory explicitly?

Well, it did so because I didn't return Null straight away when
nih_sprintfs failed in get_home_subdir() and get_user_log_dir().

> - @dir param should be const.
> - Remove comment: the directory permissions must be 0700 according
> to the spec, which is an absolute value. Since we already reset
> umask(), the code will work as expected.

Awesome. Didn't notice this.

> - This (and/or system_mkdir()) should be documented explicitly as
> only creating the "leaf" directory (ie no recursive creation).

Well... after changing get_home_subdir() & get_user_log_dir(), the
"system_mkdir()" is a wrapper with nih_assert around path argument....
I think I went overboard with this wrapper =) instead I just use mkdir
directly. I guess we could add further nih_system_error / warning
saying that we failed to create XDG paths, but imho that's just noise.