Re: [PATCH v3/GSoC 2/5] path.c: implement xdg_runtime_dir()

2016-03-26 0:55 GMT+08:00 Junio C Hamano [off-list ref]:

惠轶群 [off-list ref] writes:

quoted

quoted

There's a lot of "what" here that the caller doesn't really care about,
and which may go stale with respect to the implementation over time. Can
we make something more succinct like:

  /*
   * Return a path suitable for writing run-time files related to git,
   * or NULL if no such path can be established. The resulting string
   * should be freed by the caller.
   */

?

That's clearer, but if I were the caller, I would worry about the
security of the path.
How about adding:

The security of the path is ensured by file permission.

Is "by file permission" descriptive enough?

To protect /a/b/c/socket, what filesystem entities have the right
permission bits set?  If the parent directory is writable by an
attacker, the permission bits on 'socket' itself may not matter as
the attacker can rename it away and create new one herself, for
example.

quoted

I will deal with it.

I find there are some similar leakage in this file. I'll fix them in
another patch.

Do you think we need some additional comments for the release of strbuf?

As Documentation/technical/api-strbuf.txt has this, I think we are
already OK.

`strbuf_release`::

        Release a string buffer and the memory it used. You should not use the
        string buffer after using this function, unless you initialize it again.

Excuse me, but I could not find `Documentation/technical/api-strbuf.txt` in
master branch. Do you refer to the header of `strbuf.h`? In which, I learnt how
to initialize the strbuf and how to take use of it when I began to use
it. If there
is also a note about whether I should release it and how do it, such as:

For every strbuf that has been initialized and buffer of it has not
been detached
with strbuf_detach, you should release the resource by strbuf_release.

It will save me (maybe others) much time to explore the entire method list.