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