Re: [PATCH v2/GSoC 2/4] path.c: implement xdg_runtime_dir()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 18, 2016 at 12:48:44AM +0800, Hui Yiqun wrote:

> this function does the following:
> 
> 1. if $XDG_RUNTIME_DIR is non-empty, `$XDG_RUNTIME_DIR/git` is used in next
> step, otherwise `/tmp/git-$uid` is taken.
> 2. ensure that above directory does exist. what's more, it must has correct
> permission and ownership.
> 3. a newly allocated string consisting of the path of above directory and
> $filename is returned.
> 
> Under following situation, NULL will be returned:
> + the directory mentioned in step 1 exists but have wrong permission or
> ownership.
> + the directory or its parent cannot be created.
> 
> Notice:
> 
> + the caller is responsible for deallocating the returned string.

I see a lot of "what" in your commit message (and in the other ones in
this series), but not a lot of "why".

We can see the "what" from the diff already (though it is certainly OK
to point out tricky parts). But you probably want to explain the
motivation for things, alternatives considered, etc, like:

  - why is using $XDG_RUNTIME_DIR a good thing?

  - why did you choose to fall back to /tmp (as opposed to, say,
    returning NULL and letting the caller handle it)

  - what is the purpose of the ownership/permission rules? You link to
    the XDG spec in an in-code comment, but IMHO that kind of motivation
    probably makes more sense in the commit message.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]