RFR(S): 8176298: Log tags in -Xlog:help not sorted

[Thomas Stüfe]

Hi Marcus,



On Fri, Feb 16, 2018 at 10:09 AM, Marcus Larsson <marcus.larsson at oracle.com>
wrote:

> Hi Thomas,
>
> Thanks for reviewing!
>
>
> On 2018-02-15 18:20, Thomas Stüfe wrote:
>
>> Hi Marcus,
>>
>> I am confused, are the logtags in logTag.hpp not sorted already? So, the
>> name array should be already sorted?
>>
>
> You're right. They're currently sorted in logTag.hpp, but there's also the
> extension point LOG_TAG_LIST_EXT to extend the list with additional tags.
> Using this causes the list to be unsorted, since the tags are just appended
> to the end.
>
>
Oh, I missed that. Yes, then it makes sense.


>
>> Beside that, some small nits:
>>
>> - In LogConfiguration::print_command_line_help(), could we not use the
>> stream for the whole function? Or, better yet, make the function take an
>> outputStream* instead of a FILE*? That would fit nicely with
>> LogConfiguration::describe_available().
>>
>
> Yes, this was actually something I wanted to do, but initially thought it
> was too unrelated to the issue at hand. Now that you brought it up as well,
> I'll include that as part of this fix. :)
>
>
>> - LogTag::list_tags(): I am a tiny bit concerned about using up stack
>> space here, especially if this is used during error reporting. The array is
>> - if I am not mistaken - sizeof(enum) * ~120, so 400-500 bytes. Maybe use
>> malloc instead?
>>
>
> Hmm. I was hoping that this would be OK, but I see your concern about the
> error reporter. I found a way to do it with static storage instead, sorting
> it only once during static initialization. See if you like it, otherwise we
> can just do malloc I guess.
>
> Updated webrev:
> http://cr.openjdk.java.net/~mlarsson/8176298/webrev.01
>
> Incremental:
> http://cr.openjdk.java.net/~mlarsson/8176298/webrev.00-01
>
>
This is fine!

Thanks, Thomas


> Thanks,
> Marcus
>
>
>> Otherwise, this looks fine.
>>
>> Best Regards, Thomas
>>
>> On Thu, Feb 15, 2018 at 9:12 AM, Marcus Larsson <
>> marcus.larsson at oracle.com <mailto:marcus.larsson at oracle.com>> wrote:
>>
>>     Hi,
>>
>>     Please review the following patch to keep log tags sorted when listed.
>>
>>     Issue:
>>     https://bugs.openjdk.java.net/browse/JDK-8176298
>>     <https://bugs.openjdk.java.net/browse/JDK-8176298>
>>
>>     Webrev:
>>     http://cr.openjdk.java.net/~mlarsson/8176298/webrev.00
>>     <http://cr.openjdk.java.net/%7Emlarsson/8176298/webrev.00>
>>
>>     Verified with new unit test.
>>
>>     Thanks,
>>     Marcus
>>
>>
>>
>