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 >>>>>>>> >>>>>>> >>>>>> >>>> >> >
