Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d2621f70 authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Add protolog group id collision or duplicate check

Flag: EXEMPT adding simple assertion check
Test: atest TracingTests
Change-Id: Iac5c2e20e5e57418ef95f1f8568fd98d8d7ed613
parent 4b698405
Loading
Loading
Loading
Loading
+18 −0
Original line number Diff line number Diff line
@@ -56,6 +56,7 @@ import android.tracing.perfetto.InitArguments;
import android.tracing.perfetto.Producer;
import android.tracing.perfetto.TracingContext;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
import android.util.LongArray;
import android.util.Slog;
@@ -351,6 +352,10 @@ public class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProto
    }

    private void registerGroupsLocally(@NonNull IProtoLogGroup[] protoLogGroups) {
        // Verify we don't have id collisions, if we do we want to know as soon as possible and
        // we might want to manually specify an id for the group with a collision
        verifyNoCollisionsOrDuplicates(protoLogGroups);

        final var groupsLoggingToLogcat = new ArrayList<String>();
        for (IProtoLogGroup protoLogGroup : protoLogGroups) {
            mLogGroups.put(protoLogGroup.name(), protoLogGroup);
@@ -369,6 +374,19 @@ public class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProto
        }
    }

    private void verifyNoCollisionsOrDuplicates(@NonNull IProtoLogGroup[] protoLogGroups) {
        final var groupId = new ArraySet<Integer>();

        for (IProtoLogGroup protoLogGroup : protoLogGroups) {
            if (groupId.contains(protoLogGroup.getId())) {
                throw new RuntimeException(
                        "Group with same id (" + protoLogGroup.getId() + ") registered twice. "
                                + "Potential duplicate or hash id collision.");
            }
            groupId.add(protoLogGroup.getId());
        }
    }

    /**
     * Responds to a shell command.
     */
+2 −2
Original line number Diff line number Diff line
@@ -22,8 +22,8 @@ import com.android.internal.protolog.common.IProtoLog;
import com.android.internal.protolog.common.IProtoLogGroup;
import com.android.internal.protolog.common.LogLevel;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;

/**
 * ProtoLog API - exposes static logging methods. Usage of this API is similar
@@ -73,7 +73,7 @@ public class ProtoLog {
                if (sProtoLogInstance != null) {
                    // The ProtoLog instance has already been initialized in this process
                    final var alreadyRegisteredGroups = sProtoLogInstance.getRegisteredGroups();
                    final var allGroups = new ArrayList<>(alreadyRegisteredGroups);
                    final var allGroups = new HashSet<>(alreadyRegisteredGroups);
                    allGroups.addAll(Arrays.stream(groups).toList());
                    groups = allGroups.toArray(new IProtoLogGroup[0]);
                }
+23 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package com.android.internal.protolog;

import static org.junit.Assert.assertThrows;

import android.platform.test.annotations.Presubmit;

import com.android.internal.protolog.common.IProtoLogGroup;
@@ -44,8 +46,29 @@ public class ProtoLogTest {
                .containsExactly(TEST_GROUP_1, TEST_GROUP_2);
    }

    @Test
    public void throwOnRegisteringDuplicateGroup() {
        final var assertion = assertThrows(RuntimeException.class,
                () -> ProtoLog.init(TEST_GROUP_1, TEST_GROUP_1, TEST_GROUP_2));

        Truth.assertThat(assertion).hasMessageThat().contains("" + TEST_GROUP_1.getId());
        Truth.assertThat(assertion).hasMessageThat().contains("duplicate");
    }

    @Test
    public void throwOnRegisteringGroupsWithIdCollisions() {
        final var assertion = assertThrows(RuntimeException.class,
                () -> ProtoLog.init(TEST_GROUP_1, TEST_GROUP_WITH_COLLISION, TEST_GROUP_2));

        Truth.assertThat(assertion).hasMessageThat()
            .contains("" + TEST_GROUP_WITH_COLLISION.getId());
        Truth.assertThat(assertion).hasMessageThat().contains("collision");
    }

    private static final IProtoLogGroup TEST_GROUP_1 = new ProtoLogGroup("TEST_TAG_1", 1);
    private static final IProtoLogGroup TEST_GROUP_2 = new ProtoLogGroup("TEST_TAG_2", 2);
    private static final IProtoLogGroup TEST_GROUP_WITH_COLLISION =
            new ProtoLogGroup("TEST_TAG_WITH_COLLISION", 1);

    private static class ProtoLogGroup implements IProtoLogGroup {
        private final boolean mEnabled;