zig icon indicating copy to clipboard operation
zig copied to clipboard

`std.os.linux.ifmap` contains fields of incorrect width

Open weskoerber opened this issue 1 year ago • 6 comments

Zig Version

0.13.0-dev.211+6a65561e3

Steps to Reproduce and Observed Behavior

std.os.linux.ifmap is currently defined as:

pub const ifmap = extern struct {
    mem_start: u32,
    mem_end: u32,
    base_addr: u16,
    irq: u8,
    dma: u8,
    port: u8,
};

Note the fields mem_start and mem_end, which are defined as u32s.

On linux, struct ifmap is defined as:

struct ifmap
  {
    unsigned long int mem_start;
    unsigned long int mem_end;
    unsigned short int base_addr;
    unsigned char irq;
    unsigned char dma;
    unsigned char port;
    /* 3 bytes spare */
  };

Note the fields mem_start and mem_end are defined as unsigned long ints. On 64-bit systems, sizeof(unsigned long int) == 8.

On my 64-bit system, the u32s cause incorrect size of std.os.linux.ifmap:

  • @sizeOf(std.os.linux.ifmap) == 32
  • sizeof(struct ifmap) == 40.

This breaks most ioctls to configure network devices. I initially discovered this issue when trying to loop through each network device via the SIOCGIFCONF ioctl.

Expected Behavior

I expect the size of std.os.linux.ifmap to be identical to struct ifmap.

The C standard guarantees long is at least 32 bits but it doesn't, as far as I'm aware, make guarantees about the max size of a long. This causes the 4 byte discrepancy in each field (8 bytes total) between 32-bit and 64-bit architectures.

weskoerber avatar May 16 '24 08:05 weskoerber

I'm not totally sure how to fix this, but am willing to submit a PR. Would the following be appropriate to determine the size of the field?

    // ...
    mem_start: if (@import("builtin").cpu.arch == .x86_64) u64 else u32;
    mem_end: if (@import("builtin").cpu.arch == .x86_64) u64 else u32;
    // ...

Or would it be better to make the fields c_longs?

weskoerber avatar May 16 '24 20:05 weskoerber

I think usize is the appropriate type. C types are to be avoided if possible.

clickingbuttons avatar May 16 '24 21:05 clickingbuttons

I think usize is the appropriate type. C types are to be avoided if possible.

Does zig support 16-bit architectures? If so, wouldn't usize still be the incorrect size on 16-bit architectures (i.e. 2 bytes instead of 4)?

weskoerber avatar May 16 '24 22:05 weskoerber

possibly related: https://github.com/ziglang/zig/issues/5185

weskoerber avatar May 16 '24 22:05 weskoerber

I may be mistaken, but I don't see a 16 bit target on the tier list.

Additionally, Linux doesn't support 16 bit CPUs. You'll have to use a fork like ELKS which doesn't have a struct ifmap.

All that to say, I think usize is safe. It's used for pointer types everywhere.

clickingbuttons avatar May 17 '24 16:05 clickingbuttons

Ah ok, I understand. Thanks for the info!

weskoerber avatar May 17 '24 17:05 weskoerber