On Mon, Aug 31, 2015 at 3:29 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Sun, Aug 30, 2015 at 10:57 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote: >> On Sun, Aug 30, 2015 at 8:57 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html