Arthur Sonzogni (Gerrit)

unread,

Aug 22, 2025, 11:27:07 AMAug 22

to Siddhartha S, chromium...@chromium.org, spang...@chromium.org, wfh+...@chromium.org, tracing...@chromium.org

Attention needed from Siddhartha S

Arthur Sonzogni has uploaded the change for review

Arthur Sonzogni would like Siddhartha S to review this change.

Commit message

Replace memset with aggregate initialization in services

Using `memset` to zero-initialize C++ objects is sometimes unsafe for
non-trivial types. It bypasses C++ constructors and destructors, which
can break class invariant. Additionally, providing an incorrect size to
`memset` can result in out-of-bounds memory writes.

This patch replaces these calls with aggregate initialization
(`instance = {}`), which correctly and safely initializes members
without bypassing C++ language semantics.

The bulk of this patch was generated by a script using tree-sitter:
https://paste.googleplex.com/5357187081437184?raw
The script was mainly generated by gemini.

Following the automated pass, Gemini was prompted to refine the changes
by:
- Reverting changes for most structs containing unions.
- Reverting changes where nearby comments indicated `memset` was
  intentional.
- Moving initializations directly to the declaration where possible.

Reviewer Note: This refactoring is intended to be a behavior-preserving
modernization. However, `memset` and aggregate initialization are not
strictly equivalent. For instance `memset` zeroes the entire memory
block, including padding, while aggregate initialization only
initializes specified members. Please review carefully for any code that
might have implicitly relied on the non-standard behavior of zeroing the
entire object memory. I did my past to spot such cases, but there may be
some edge cases that require a review.

This CL was uploaded by git cl split.

R=ss...@chromium.org

Bug: 435317390

Change-Id: Ia7b8a0994bc6d55862e91e2f5ddbca530e42c2ac

Change diff

diff --git a/services/tracing/public/cpp/system_tracing_service.cc b/services/tracing/public/cpp/system_tracing_service.cc
index b2374bf1..3edb220 100644
--- a/services/tracing/public/cpp/system_tracing_service.cc
+++ b/services/tracing/public/cpp/system_tracing_service.cc
@@ -75,7 +75,7 @@
     }
 
     struct sockaddr_un saddr;
-    UNSAFE_TODO(memset(&saddr, 0, sizeof(saddr)));
+    saddr = {};
     UNSAFE_TODO(memcpy(saddr.sun_path, producer_sock_name.data(),
                        producer_sock_name.size()));
     saddr.sun_family = AF_UNIX;

Change information

Files:

M services/tracing/public/cpp/system_tracing_service.cc

Change size: XS

Delta: 1 file changed, 1 insertion(+), 1 deletion(-)

Open in Gerrit

Related details

Attention is currently required from:

Siddhartha S

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Arthur Sonzogni (Gerrit)

unread,

Aug 22, 2025, 12:18:27 PMAug 22

to AyeAye, Siddhartha S, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Attention needed from Siddhartha S

Arthur Sonzogni voted Auto-Submit+0

Auto-Submit

+0

Open in Gerrit

Related details

Attention is currently required from:

Siddhartha S

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Arthur Sonzogni (Gerrit)

unread,

Aug 25, 2025, 8:05:08 AMAug 25

to AyeAye, Siddhartha S, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Attention needed from Siddhartha S

Arthur Sonzogni voted and added 1 comment

Votes added by Arthur Sonzogni

Auto-Submit	+1
Commit-Queue	+1

1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Arthur Sonzogni . resolved

Hi Siddhartha, could you please take a look?

Open in Gerrit

Related details

Attention is currently required from:

Siddhartha S

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Siddhartha S (Gerrit)

unread,

Aug 25, 2025, 4:36:56 PMAug 25

to Arthur Sonzogni, Siddhartha S, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Attention needed from Arthur Sonzogni

Siddhartha S voted

Code-Review	+1
Commit-Queue	+2

Open in Gerrit

Related details

Attention is currently required from:

Arthur Sonzogni

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Aug 25, 2025, 4:42:31 PMAug 25

to Arthur Sonzogni, Siddhartha S, AyeAye, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6875522

Auto-Submit: Arthur Sonzogni <arthurs...@chromium.org>

Commit-Queue: Siddhartha S <ss...@chromium.org>

Reviewed-by: Siddhartha S <ss...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1506106}

Files:

M services/tracing/public/cpp/system_tracing_service.cc

Change size: XS

Delta: 1 file changed, 1 insertion(+), 2 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Siddhartha S

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Tom Sepez (Gerrit)

unread,

Aug 25, 2025, 4:48:00 PMAug 25

to Arthur Sonzogni, Chromium LUCI CQ, Siddhartha S, AyeAye, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Attention needed from Arthur Sonzogni

Tom Sepez added 1 comment

File services/tracing/public/cpp/system_tracing_service.cc

Line 78, Patchset 3 (Latest):

    UNSAFE_TODO(memcpy(saddr.sun_path, producer_sock_name.data(),
                       producer_sock_name.size()));

Tom Sepez . unresolved

Follow-on: Do we rewrite memcpy in terms of:
```
base::byte_span_from_ref(saddr).copy_prefix_from(base::as_byte_span(producer_sock_name));
```

Open in Gerrit

Related details

Attention is currently required from:

Arthur Sonzogni

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Aug 25, 2025, 5:20:26 PMAug 25

to Arthur Sonzogni, Chromium LUCI CQ, Siddhartha S, AyeAye, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Attention needed from Arthur Sonzogni

Tom Sepez added 1 comment

File services/tracing/public/cpp/system_tracing_service.cc

Line 78, Patchset 3 (Latest):

    UNSAFE_TODO(memcpy(saddr.sun_path, producer_sock_name.data(),
                       producer_sock_name.size()));

Tom Sepez . resolved

Follow-on: Do we rewrite memcpy in terms of:
```
base::byte_span_from_ref(saddr).copy_prefix_from(base::as_byte_span(producer_sock_name));
```

Tom Sepez

Done

Open in Gerrit

Related details

Attention is currently required from:

Arthur Sonzogni

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Arthur Sonzogni (Gerrit)

unread,

Aug 26, 2025, 11:00:26 AMAug 26

to Chromium LUCI CQ, Tom Sepez, Siddhartha S, AyeAye, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Arthur Sonzogni added 1 comment

File services/tracing/public/cpp/system_tracing_service.cc

Line 78, Patchset 3 (Latest):

    UNSAFE_TODO(memcpy(saddr.sun_path, producer_sock_name.data(),
                       producer_sock_name.size()));

Tom Sepez . resolved

Follow-on: Do we rewrite memcpy in terms of:
```
base::byte_span_from_ref(saddr).copy_prefix_from(base::as_byte_span(producer_sock_name));
```

Tom Sepez

Done

Arthur Sonzogni

Thanks!

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Replace memset with aggregate initialization in services [chromium/src : main]

Arthur Sonzogni (Gerrit)

Arthur Sonzogni has uploaded the change for review

Commit message

Change diff

Change information

Related details

Arthur Sonzogni (Gerrit)

Arthur Sonzogni voted Auto-Submit+0

Related details

Arthur Sonzogni (Gerrit)

Arthur Sonzogni voted and added 1 comment

Votes added by Arthur Sonzogni

1 comment

Related details

Siddhartha S (Gerrit)

Siddhartha S voted

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Tom Sepez (Gerrit)

Tom Sepez added 1 comment

Related details

Tom Sepez (Gerrit)

Tom Sepez added 1 comment

Related details

Arthur Sonzogni (Gerrit)

Arthur Sonzogni added 1 comment

Related details