Hi Aaron,
On Fri, 2020-02-21 at 12:21 -0500, Aaron Merey wrote:
> On Fri, Feb 21, 2020 at 11:13 AM Mark Wielaard <m...@klomp.org> wrote:
> > On Wed, 2020-02-19 at 16:04 -0500, Aaron Merey wrote:
> > > + /* Create XDG cache directory if it doesn't exist. */
> > > + if (stat (cachedir, &st) == 0)
> > > + {
> > > + if (! S_ISDIR (st.st_mode))
> > > + {
> > > + rc = -EEXIST;
> > > + goto out;
> > > + }
> > > + }
> > > + else
> > > + {
> > > + char *cmd;
> > > +
> > > + xalloc_str (cmd, "mkdir -p \'%s\'", cachedir);
> > > + rc = system (cmd);
> >
> > Please don't use system. Use rc = mkdir (cachedir, 0700);
>
> Sure. My goal was to have any nonexisting parent directories created as
> easily as possible and I wasn't aware of anything besides `mkdir -p`.
> Should we not bother creating nonexisting parent directories?
I think the spec is slightly ambiguous whether or not you need to
create all parents.
https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
If, when attempting to write a file, the destination directory is
non-existant an attempt should be made to create it with permission
0700.
But I think that means we should only create the cachedir (either
$XDG_CACHE_HOME or $HOME/.cache). Which is what we are doing here. But
I don't think we are expected to create all parents. I think it is
reasonable to assume they already exist (it would almost always be
$HOME anyway).
Later in the code we create target_cache_path (the debuginfod_client
subdir), again just as one directory (we made sure its parent exists).
BTW. Looking at the code again I see this comment should be adjusted
too:
/* set paths needed to perform the query
example format
cache_path: $HOME/.debuginfod_cache
target_cache_dir: $HOME/.debuginfod_cache/0123abcd
target_cache_path: $HOME/.debuginfod_cache/0123abcd/debuginfo
target_cache_path:
$HOME/.debuginfod_cache/0123abcd/source#PATH#TO#SOURCE ?
*/
Cheers,
Mark
