Staffan and David, thanks for your reviews. /R
On Apr 26, 2013, at 2:57 PM, Staffan Larsen wrote: > Looks good. > > /Staffan > > On 26 apr 2013, at 07:31, Rickard Bäckman <rickard.back...@oracle.com> wrote: > >> 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 >>>>>>>>> >>>>>>>> >>>>>>> >>>>> >>> >> >
