Do *not* truncate tags over 23 characters

[consp1racy]

Quoting my comment #196 (comment)

Log doesn't care at all how long the tags are.

The only thing where is an issue is that Log.isLoggable checks a system property. And system property keys were until API 24 limited in length to 31 characters. And "log.tag.".size + 23 equals 31.

Source:

• https://android.googlesource.com/platform/frameworks/base/+/kitkat-release/core/jni/android_util_Log.cpp
• https://android.googlesource.com/platform/frameworks/base/+/oreo-release/core/jni/android_util_Log.cpp

Going forward LogcatTree could

• drop this limitation entirely or
• enforce it for one or both of checking and printing just when withCompliantLogging() is used.

[consp1racy]

I just combed through AOSP API 25 and the check is still in place. The documentation for Log.isLoggable got updated in Oreo with the wrong thing. Looks like we should be checking tag length until API 26.

Folow https://issuetracker.google.com/issues/124593220

[JakeWharton]

This behavior already exists:

timber/timber/src/main/java/timber/log/Timber.java

Lines 586 to 589 in 84b1576

	// Tag length limit was removed in API 24.
	if (tag.length() <= MAX_TAG_LENGTH || Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
	return tag;
	}

[consp1racy]

You missed the point. isLoggable isn't safe. Actual logging is.

And the second point, it isn't safe up until and including API 25. The documentation is wrong.

[JakeWharton]

We don't call that API.

[consp1racy]

Ok, in which case we don't have to truncate the tags at all. The only thing that throws is Log.isLoggable below API 26. Again, the actual logging doesn't care about tag length (according to AOSP).

Plus the new LogcatTree.withCompliantLogging() does call Log.isLoggable.

I probably should draft a sample project.

[JakeWharton]

The struct sent to the kernel only had 23 bytes until API 24

…

On Mon, Feb 18, 2019, 9:13 AM Eugen Pechanec ***@***.*** wrote: Ok, in which case we don't have to truncate the tags at all. The only thing that throws is Log.isLoggable below API *26*. Again, the actual logging doesn't care about tag length (according to AOSP). Plus the new LogcatTree.withCompliantLogging() does call Log.isLoggable. I probably should draft a sample project. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#350 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEYBvampeA24OY6JuDPrkcQ__wft-ks5vOrUjgaJpZM4bAbtw> .

[consp1racy]

Admittedly, C is not my forte... Anyway, how is it then possible that logging with tags over 23 characters just works everywhere I tried?

I took a Huawei ALE-L21 with Android 6, whipped out a Samsung Galaxy SII running Android 4.1, and unearthed a HTC Vision with a dusty Gingerbread. All of them log just fine, until I call Log.isLoggable.

And on the other side we have my old Xperia L with an unofficial Android 7.1 that throws IllegalArgumentException while according to the documentation logn tags should be allowed.

Here's the sample project: https://github.com/consp1racy/android-test-loggable

Here's a sample input:

Log.w("The wheels on the bus go round and round, round and round, round and round", "Hello, World!")

Here's a sample output:

01-06 01:11:29.321 2343-2343/net.xpece.test.loggable W/The wheels on the bus go round and round, round and round, round and round: Hello, World!

Bottom line:

• Log.isLoggable documentation is wrong. It checks tag length until and including API 25. This is important for the new LogcatTree.withCompliantLogging.
• Log.w logs long tags just fine on everything I tested. This is important for the whole Timber.

Another logging library has observed the same thing: https://jira.qos.ch/browse/SLF4J-164

I wouldn't even know that too long tags was a thing until one day Timber just started truncating them out of the blue. Everything worked fine.

The struct sent to the kernel only had 23 bytes until API 24

Why does it still work? It prints the tag from unallocated memory until it encounters a \0? What's happening there? 23 bytes isn't even 23 wide characters.

[JakeWharton]

I don't see a reason to bother with changing this. The limitation is removed on new API levels and the restriction was widely understood to exist prior to that. Whether it actually is enforced and where is an implementation detail and I don't want to mess with it.