Re: [PATCH v14 04/13] ref-filter: implement an `align` atom

From: Karthik Nayak <karthik.188@gmail.com>
Date: Aug 30, 2015, 14:57 UTC
Message-ID: <CAOLa=ZRZ6RcRm6ERR=NVzng17qJx+1N8iGThuo=xQhEszJQ90w@mail.gmail.com>
In-Reply-To: <CAPig+cRHRPehkd+9PwOqpXkRUvaJa42zLtCKMEfv2W=ZJUZJzA@mail.gmail.com>

On Sun, Aug 30, 2015 at 8:57 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 33 quoted lines

>> Add documentation and tests for the same.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 432cea0..21c8b5f 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -53,6 +54,13 @@ static struct {
>>         { "flag" },
>>         { "HEAD" },
>>         { "color" },
>> +       { "align" },
>> +       { "end" },
>> +};
>> +
>> +struct align {
>> +       align_type position;
>> +       unsigned int width;
>>  };
>>
>>  #define REF_FORMATTING_STATE_INIT  { 0, NULL }
>> @@ -69,6 +79,8 @@ struct ref_formatting_state {
>>
>>  struct atom_value {
>>         const char *s;
>> +       struct align *align;
>
> Why does 'align' need to be heap-allocated rather than just being a
> direct member of 'atom_value'? Does 'align' need to exist beyond the
> lifetime of its 'atom_value'? If not, making it a direct member might
> simplify resource management (no need to free it).
>

But it does, since we carry over the contents of align from atom_value to cb_data of ref_formatting_stack and that holds the value until we read the %(end) atom hence it seemed like a better choice to allocate it on the heap

-- 
Regards,
Karthik Nayak