Re: [PATCH 3/5] sequencer: sequencer state is useless without todo

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Nov 15, 2011, 09:13 UTC
Message-ID: <CALkWK0nUuzn2_itdACHLQBpUaVv97tFAjNGdVBEhWC7a6Rp75w@mail.gmail.com>
In-Reply-To: <7v7h33oifq.fsf@alter.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:

Show 14 quoted lines

> 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