All commit authors signed the Contributor License Agreement.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

I'm thinking this probably won't need a news entry since it's just fixing a small bug that's been outstanding for quite a while.

But I'm sure it will affect someone's workflow.

If it's a user-visible change - it worth a news entry.

The implementation looks pretty good, apart from my nitpicks. This needs a test though, could you add one somewhere in test_ctypes? (You should be able to just retrofit the reproducer from the issue as a test.)

Hey @ZeroIntensity, thanks for the review! This is my first time contributing to Python so I'm still getting a feel for how this project works. I just added some tests and cleaned up the news blurb.

As I noted on the issue, c_bool not being swappable is intended behaviour -- like pointers, it's a platform-specific type.

As far as I know, basically all systems interpret a bool as 1 byte, but I guess it's not always true. Why not just apply the __ctype_le__ and __ctype_be__ when sizeof(c_bool) == 1?

Then, using c_bool would break on some platforms.

If you know your bools are single bytes, you should use c_uint8 (or its synonym c_byte) -- that behaves the same everywhere.
Is there a use case where you can't do this?

It would break, but I don't think c_int8 is much better--if you put that in a struct, that would crash on all the systems that don't have single-byte bools. Opposite endian support here is sort of a nice convenience thing for when you know that it works.

that would crash

What would crash?

You can't really use the struct in C, where working with it would crash. Non-native endianness is mostly useful for data sent from another system. (Arguably it would fit better in struct rather than ctypes, but let's not go there today.)

For data interchange, you really should be using fixed-width types, e.g. int32_t, rather than platform-specific ones like int. The latter would “crash” on other systems.
In ctypes, mainly for historical reasons AFAIK, c_int32 is a synonym for c_int (or similar). If they were separate types, __ctype_le__ and __ctype_ge__ would, IMO, only make sense for c_int32.

What would crash?

Using a c_int8 in place of a c_bool somewhere would segfault if bool isn't a single byte, wouldn't it?

I can't say much about use cases: I haven't ever used ctypes for data exchange, so I'd be mostly speculating. I'd like to hear from @zbarnett before closing this.

Sorry for dropping the ball here.

Using a c_int8 in place of a c_bool somewhere would segfault if bool isn't a single byte, wouldn't it?

Generally yes, but here, “somewhere” is inside a struct with a non-native byte order. That is, a struct that's not meant for the current architecture. You can't quite use it from C.

I'd like to hear from @zbarnett before closing this.

Same here. @zbarnett, do you have a use case for this?

I appreciate the time you both have taken to think through this.

I'm using ctypes to read/write data across the network to other systems that have opposite endianness. I am currently using c_uint8 as a workaround but it's a bit clunky having to deal with 0 and 1 instead of False and True. In almost all places this is being used, I'm also setting the bit length to 1 and it's often not aligned on a byte boundary anyway.

Regarding portability, aren't all the builtin types in ctypes provided assuming some things are constant (like bit endianness) that aren't actually 100% constant across all systems? I had, probably wrongly, assumed that a bool would almost always be represented by 8 bits. But I do see the value in explicitly defining the size in bits of all the types when interfacing with other systems. In that case, would we want something like c_bool1, c_bool8, c_bool32, etc. to allow a precise size to be defined but still allow usage of True/False in the Python code?

Either way, I would assume that if c_int supports other endian, c_bool should too. If anything, c_bool would be less problematic than c_int in most cases.

Thanks for the input!

Yes, I think c_bool1, c_bool8, c_bool32, etc. would fit better than adding c_bool.__ctype_le__, but we can do even better (eventually).
The feature request is similar to wanting to convert a C integer to a Python IntEnum. Both could be served by custom getter/setter conversion functions (here, ideally something as simple as getter=bool), so we decouple the representation of the C integer from how it's mapped to a Python object.

In almost all places this is being used, I'm also setting the bit length to 1 and it's often not aligned on a byte boundary anyway.

You also want c_uint8 for that. In the ms_struct layout, the offsets depend on the size of the type, not just the bit length of the field.

I'm sorry for complicating your simple use case, but if we add something that's hard to generalize, we'll need to deal with the edge cases piling up later :(

If anything, c_bool would be less problematic than c_int in most cases.

Unfortunately, bool is a can of worms -- for cross-platform data exchange at least.