I've updated the webrev (empty struct option). http://cr.openjdk.java.net/~rbackman/8013117.u1/
Thanks /R On Apr 24, 2013, at 2:40 PM, Rickard Bäckman wrote: > David, > > On Apr 24, 2013, at 2:25 PM, David Holmes wrote: > >> On 24/04/2013 9:05 PM, Rickard Bäckman wrote: >>> David, >>> >>> On Apr 24, 2013, at 12:11 PM, David Holmes wrote: >>> >>>> On 24/04/2013 7:09 PM, Rickard Bäckman wrote: >>>>> Nils, >>>>> >>>>> no it doesn't matter. Rather intended. By initializing it to NULL we >>>>> forced implementors to use a pointer that would have to be initialized at >>>>> some point. Now it can be a class / struct >>>>> that is instead initialized by a default constructor. >>>> >>>> So that addressed my question on the missing setter. But doesn't this also >>>> mean that you are now prohibiting it from being a simple pointer-type as >>>> there is no way to set it? Isn't maintaining the setter more flexible as >>>> it can be used in either case (direct assignment or copy constructor). >>>> Though lack of initialization in the current code still looks wrong. >>> >>> Yes it makes it harder for the type to be a simple pointer, though that >>> could be worked around by putting the pointer inside a struct. Not a great >>> solution perhaps. >>> The other solution would be to have a setter, the question is what to >>> initialize it with? Should we add another hook? >> >> I don't see a truly satisfactory solution, but I think Parfait will flag >> that you are returning an uninitialized variable here. > > Agreed. > >> >> So yes perhaps you need to define TRACE_DATA_INIT ? > > Two options I think: > > 1) restore the setter and create a macro TRACE_DATA_DEFAULT_VALUE > 2) make TRACE_DATA an empty struct. > > Both works for me. > > /R > >> >> David >> >>> /R >>> >>>> >>>> David >>>> >>>>> /R >>>>> >>>>> On Apr 24, 2013, at 10:45 AM, Nils Loodin wrote: >>>>> >>>>>> Does it matter that the pointer gets initialized to NULL before, but not >>>>>> now? There isn't any null checks anywhere that depends on that? >>>>>> >>>>>> Regards, >>>>>> Nils >>>>>> >>>>>> On 04/24/2013 09:51 AM, Rickard Bäckman wrote: >>>>>>> Hi all, >>>>>>> >>>>>>> can I have a couple of reviews for this small change. The short story >>>>>>> is that the current way the thread-local _trace_buffer is somewhat >>>>>>> inflexible. >>>>>>> By changing the type of the getter this structure gets more flexible >>>>>>> for different implementations. I also think that the name is misused. >>>>>>> Just naming it >>>>>>> to _trace_data is more generic and less implementation-specific. >>>>>>> >>>>>>> The webrev: http://cr.openjdk.java.net/~rbackman/8013117/ >>>>>>> >>>>>>> Thanks >>>>>>> /R >>>>>>> >>>>>> >>>>> >>> >
