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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 30, 2015, 21:59 UTC
Message-ID: <CAPig+cRUJa3hRk4utH6A8iHv6y2=XukTkiLWFet4dkeEw1sLzA@mail.gmail.com>
In-Reply-To: <CAOLa=ZRZ6RcRm6ERR=NVzng17qJx+1N8iGThuo=xQhEszJQ90w@mail.gmail.com>

On Sun, Aug 30, 2015 at 10:57 AM, Karthik Nayak <karthik.188@gmail.com> wrote:

Show 22 quoted lines

> On Sun, Aug 30, 2015 at 8:57 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> +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

So, you're saying that the 'atom_value' instance no longer exists at the point that processing of %(end) needs to access the alignment properties? If so, then heap allocation make sense. Thanks.