This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] debuginfod-client: default to XDG cache.


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 <mark@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]