On Mon, Aug 31, 2015 at 3:29 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Sun, Aug 30, 2015 at 10:57 AM, Karthik Nayak <karthik....@gmail.com> wrote:
>> On Sun, Aug 30, 2015 at 8:57 AM, Eric Sunshine <sunsh...@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.

I was actually wrong there, if you see populate_value() the ref is
filled with atoms
which aren't really deallocated, hence the atom_value remains with the ref in
ref->value[atom]. where atom is obtained using parse_ref_filter_atom() hence it
makes sense to make it static. Thanks

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to