Re: [PATCH] branch: colour upstream branches

> On Sun, Apr 14, 2013 at 7:55 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Mon, Apr 15, 2013 at 9:54 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Sun, Apr 14, 2013 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>>>> On Mon, Apr 15, 2013 at 9:22 AM, Felipe Contreras
>>>> <felipe.contreras@gmail.com> wrote:
>>>>> On Sun, Apr 14, 2013 at 5:31 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>>>>>> On Sun, Apr 14, 2013 at 11:46 AM, Felipe Contreras
>>>>>> <felipe.contreras@gmail.com> wrote:
>>>>>>> +       char fancy[80];
>>>>>>>
>>>>>>> -       if (show_upstream_ref)
>>>>>>> +       if (show_upstream_ref) {
>>>>>>>                 ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
>>>>>>> +               if (want_color(branch_use_color))
>>>>>>> +                       snprintf(fancy, sizeof(fancy), "%s%s%s",
>>>>>>> +                                       GIT_COLOR_BLUE, ref, GIT_COLOR_RESET);
>>>>>>> +               else
>>>>>>> +                       strncpy(fancy, ref, sizeof(fancy));
>>>>>>> +       }
>>>>>>> +
>>>>>>
>>>>>> Please use strbuf for "fancy".
>>>>>
>>>>> Why? We would need to initialize and free it. What's the advantage?
>>>>
>>>> From a quick glance, I don't see any gurantee that "ref" (plus ansi
>>>> codes) will always fit in 80 bytes.
>>>
>>> Would changing it to 1024 (MAXREFLEN) fix it?
>>
>> You still need to take ansi codes into account. I think it's easier to
>> just use strbuf.
>
> I'm not sure what you mean. If there was an issue with snprintf, then
> there would be with this in refs.c:
>
>         if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)