From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Tue, 15 Nov 2011 09:13:33 GMT
Subject: Re: [PATCH 3/5] sequencer: sequencer state is useless without todo
Message-ID: <CALkWK0nUuzn2_itdACHLQBpUaVv97tFAjNGdVBEhWC7a6Rp75w@mail.gmail.com>
In-Reply-To: <7v7h33oifq.fsf@alter.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>>>>  static int create_seq_dir(void)
>>>>  {
>>>> +     const char *todo_file = git_path(SEQ_TODO_FILE);
>>>>       const char *seq_dir = git_path(SEQ_DIR);
>>>
>>> Scary idiom.
>>
>> What's scary about it?
>
> The next person who copies and pastes this code to other codepaths without
> thinking that the return value of git_path() is ephemeral and may need to
> be saved away depending on what goes between its assignment and its use.

Yeah, git_path() writes to one of the four static buffers in
path.c:get_pathname().  Which brings me to: what should (can) we do
about it?  Explicitly xmalloc()'ing and free()'ing a tiny path buffer
is an overkill, so I'm thinking more on the lines of good
documentation.  I've been guilty of misusing git_path() blindly in the
past myself.

Thanks.

-- Ram