Skip to content

_remote_debugging: magic and duplicate consts in binary format helpers #149300

@maurycy

Description

@maurycy

Feature or enhancement

Proposal:

This is a follow up to #148972 (comment)

In short, binary reader and writer could use some small clean up of consts.

There are some duplicates:

#define MAX_DECOMPRESS_SIZE (1ULL << 30)

#define MAX_DECOMPRESS_SIZE (1ULL << 30)

There are some magic numbers:

uint64_t file_size = (uint64_t)footer_offset + 32;
uint8_t footer[32] = {0};
/* Cast size_t to uint32_t before memcpy to ensure correct bytes are copied
* on both little-endian and big-endian systems (size_t is 8 bytes on 64-bit) */
uint32_t string_count_u32 = (uint32_t)writer->string_count;
uint32_t frame_count_u32 = (uint32_t)writer->frame_count;
memcpy(footer + 0, &string_count_u32, 4);
memcpy(footer + 4, &frame_count_u32, 4);
memcpy(footer + 8, &file_size, 8);

There are definitely places in the module that handle it better, eg:

uint8_t header[FILE_HEADER_SIZE] = {0};
uint32_t magic = BINARY_FORMAT_MAGIC;
uint32_t version = BINARY_FORMAT_VERSION;
memcpy(header + HDR_OFF_MAGIC, &magic, HDR_SIZE_MAGIC);
memcpy(header + HDR_OFF_VERSION, &version, HDR_SIZE_VERSION);
header[HDR_OFF_PY_MAJOR] = PY_MAJOR_VERSION;
header[HDR_OFF_PY_MINOR] = PY_MINOR_VERSION;
header[HDR_OFF_PY_MICRO] = PY_MICRO_VERSION;
memcpy(header + HDR_OFF_START_TIME, &writer->start_time_us, HDR_SIZE_START_TIME);
memcpy(header + HDR_OFF_INTERVAL, &writer->sample_interval_us, HDR_SIZE_INTERVAL);
memcpy(header + HDR_OFF_SAMPLES, &writer->total_samples, HDR_SIZE_SAMPLES);
memcpy(header + HDR_OFF_THREADS, &thread_count_u32, HDR_SIZE_THREADS);
memcpy(header + HDR_OFF_STR_TABLE, &string_table_offset_u64, HDR_SIZE_STR_TABLE);
memcpy(header + HDR_OFF_FRAME_TABLE, &frame_table_offset_u64, HDR_SIZE_FRAME_TABLE);
memcpy(header + HDR_OFF_COMPRESSION, &compression_type_u32, HDR_SIZE_COMPRESSION);
if (fwrite_checked_allow_threads(header, FILE_HEADER_SIZE, writer->fp) < 0) {

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

Metadata

Metadata

Assignees

No one assigned

    Labels

    extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions