Reduce duplicated code - #243
Merged
Merged
Conversation
fuse_write_all looped "while (len > 0)" and, on a write() that returned 0, advanced by nothing and spun forever instead of reporting an error. Materializing a FUSE file for exec could therefore hang the vCPU thread. Use write_all, which treats an unexpected zero-byte write as EIO. This also removes a private copy of the same write loop.
sidecar_write_all was a second copy of utils.h's write_all with the same short-write and EINTR handling. Call write_all instead.
align2m_up and align2m_down duplicated the ALIGN_2MIB_UP and ALIGN_2MIB_DOWN macros in utils.h, and two more sites open-coded the "& ~(BLOCK_2MIB - 1)" mask. Use the macros so 2 MiB rounding has one definition.
The FIONBIO handler open-coded an F_GETFL / modify / F_SETFL sequence, and two pty-keepalive paths open-coded F_GETFD / F_SETFD | FD_CLOEXEC. utils.h already provides fd_update_status_flag and fd_set_cloexec for exactly these; use them.
Five component walks in path.c and sidecar.c open-coded the same block:
declare a NAME_MAX+1 buffer, reject an overlong component with
ENAMETOOLONG, then memcpy the counted component and NUL-terminate. The
bound check was even spelled two ways ("len > NAME_MAX" and
"comp_len >= sizeof(buf)").
Fold that into a path_component_copy helper next to path_next_component,
which produces the counted, non-NUL-terminated components these sites
consume. Callers keep their own cleanup on the error path and gate it on
the helper's return.
gdbstub-rsp.c (hex_val) and sidecar.c (hex_nibble) each carried the same hex-digit decode ladder, the inverse of the bytes_to_hex encoder already in utils.h. Move it there as hex_nibble and drop both copies. gdb_hex_encode already uses bytes_to_hex, so the two now share both directions of the codec.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes six instances of duplicated logic across the codebase,
folding each into a single shared definition. No behavior changes except where a
duplicate copy had a latent bug (see the FUSE write fix below).
Motivation
Several small utilities — a hex-nibble decoder, a counted path-component copy,
fd-flag manipulation, 2 MiB alignment, and a
write_allloop — had beenreimplemented independently in two or more files. Each copy is a place a bug can
hide or the two can drift apart (one already had). Consolidating them gives every
call site one definition to read and maintain.
Changes
Each commit is a self-contained consolidation:
fuse_write_allloopedwhile (len > 0)and, on awrite()returning 0, advanced by nothing and spunforever instead of erroring. Materializing a FUSE file for exec could hang the
vCPU thread. Replaced with the shared
write_all, which treats an unexpectedzero-byte write as
EIO.sidecar_write_allfor the sharedwrite_all— removed a second copyof
utils.h'swrite_allwith identical short-write /EINTRhandling.ALIGN_2MIBmacros in the Rosetta loader —align2m_up/align2m_downduplicatedALIGN_2MIB_UP/ALIGN_2MIB_DOWN, and two moresites open-coded the
& ~(BLOCK_2MIB - 1)mask. Now 2 MiB rounding has onedefinition.
FIONBIOhandler open-coded an
F_GETFL/ modify /F_SETFLsequence, and twopty-keepalive paths open-coded
F_GETFD/F_SETFD | FD_CLOEXEC. Replacedwith the existing
fd_update_status_flagandfd_set_cloexec.path_component_copyfor counted path components — five componentwalks in
path.candsidecar.copen-coded the sameNAME_MAX+1buffer /ENAMETOOLONGcheck /memcpy+ NUL-terminate block (with the bound checkspelled two different ways). Folded into a
path_component_copyhelper next topath_next_component.hex_nibbleas the companion tobytes_to_hex—gdbstub-rsp.c(
hex_val) andsidecar.c(hex_nibble) each carried the same hex-digitdecode ladder, the inverse of the existing
bytes_to_hexencoder. Moved toutils.hso both directions of the codec live together.Summary by cubic
Consolidates six duplicated helpers into shared utilities, reducing drift and simplifying maintenance. Also fixes a vCPU hang by failing FUSE materialization on zero‑progress writes; no other behavior changes.
Bug Fixes
Refactors
ALIGN_2MIB_UP/ALIGN_2MIB_DOWNin Rosetta; remove open‑coded masks.fcntlsequences withfd_set_cloexecandfd_update_status_flag.path_component_copyand use it for counted components inpath.candsidecar.c.hex_nibbletoutils.h; reuse ingdbstub-rspandsidecar.sidecar_write_alland call sharedwrite_all.Written for commit 76d9378. Summary will update on new commits.