1. 20
  1. 2

    That’s … not idiomatic C code there. I would encode his TIFF example as:

    typedef enum
    {
      TIFFEntryValueTagSmall,
      TIFFEntryValueTagBlock,
    } TIFFType__e;
    
    typedef struct
    {
      TIFFType__e type;
      uint32_t    value;
    } TIFFEntryValueTagSmall__s;
    
    typedef struct
    {
      TIFFType__e type;
      size_t      size;
      void const *data;
    } TIFFEntryValueTagBlock__s;
    
    typedef union
    {
      TIFFType__e               type;
      TIFFEntryValueTagSmall__s small;
      TIFFEntryValueTagBlock__s block;
    } TIFFEntry__u;
    
    inline static TIFFEntry__u TIFFEntryValueSmall(value)
    {
      return (TIFFEntry__u) {
        .small = { .type = TIFFEntryValueTagSmall , .value = value }
      };
    }
    
    inline static TiffEntry__u TIFFEntryValueBlock(size_t size,void const *ptr)
    {
      return (TIFFEntry__u) {
        .block = { .type = TIFFEntryValueTagBlock , .size = size , .data = ptr }
      };
    }
    

    That way, a type of TIFFEntryValueSmall does not have access to the data or size fields of the TIFFEntryValueTagBlock. And it does not involve adding new syntax to C macros.

    1. 1

      Considering the replies you got, I don’t think there is such a thing as idiomatic C. The language doesn’t specify much, and I doubt a style is included. It seems to be what platform and project you’re used to. I’m sure if you asked a Win32 weenie you’d get plenty of struct tagTIFFDATA { LPVOID lpvData; ...}.

      1. 1

        That’s … not idiomatic C code there

        That’s how I would write it. (Assuming you’re talking about the original code with the anonymous union; not the macro soup, which is admittedly impressive.) Your code takes advantage of aliasing and, though not technically undefined behaviour (a struct is allowed to alias its first member), doesn’t sit well and is much easier to mess up.

        1. 1

          I totally agree with that. As I said in my other reply, it’s also just superfluous to define so much boilerplate for what effectively ends up in a controlled-initialization anyway.

          1. 1

            Perhaps I’ve done too much Xlib programming and dealing with XEvents (which is done in the manner I presented), but I prefer that approach, as it makes clear which TIFF objects have which fields (add about half a dozen more TIFF types and see how clear the original remains), and allows one to write functions that take particular TIFF types (if that makes any sense). It’s not always about initialization, but usage as well.

            1. 3

              it makes clear which TIFF objects have which fields (add about half a dozen more TIFF types and see how clear the original remains), and allows one to write functions that take particular TIFF types

              It depends highly on the specific situation and how large the types are, but I frequently define intermediate structure types. I just don’t like aliasing the first member. Something like this:

              typedef enum {
              	TiffEntry_Small,
              	TiffEntry_Block,
              } TiffEntryType;
              
              typedef struct {
              	const void *ptr;
              	size_t len;
              } TiffEntryBlock;
              
              typedef struct {
              	TiffEntryType type;
              	union {
              		uint32_t small;
              		TiffEntryBlock block;
              	};
              } TiffEntry;
              
              TiffEntry tiff_entry_make_small(uint32_t value) {
              	return (TiffEntry){.type=TiffEntry_Small, .small=value};
              }
              
              TiffEntry tiff_entry_make_block(void *ptr, size_t len) {
              	return (TiffEntry){.type=TiffEntry_Block, .block={.ptr=ptr, .len=len}};
              }
              

              (I probably wouldn’t define a whole new structure type for something with only 2 members; but that example is still illustrative, and I would almost certainly do it for one that had ≥3 or 4.)


              Ultimately, the problem I have with structures like TIFFEntryValueTagSmall__s is that it’s possible to construct a TIFFEntryValueTagSmall__s which is not tagged with TIFFEntryValueTagSmall. That makes the whole structure very brittle. Whereas something like my TiffEntryBlock is always correct on its own, and can trivially be built up into a TiffEntry when the generic structure is needed.

        2. 1

          Talking about idiomatic code (of which there is no one true definition), I personally don’t like your __-suffixes indicating the type while typedeffing them. We can surely have a debate on that, but I think you should only typeset when dealing with an opaque type (and I mean opaque as in FILE or something; the type-field in tiff_entry below is still useful for the normal “consumer” and thus the type is not opaque). Given you prefix anything with “struct”, “enum” or “union” otherwise anyway (by language norm), you are solving a problem with the typedefs that you introduced in the first place. The direct struct-return is a bit iffy as well and I’d rather define a function that is passed a pointer and a value to fill.

          In total, I’d go with the following approach. There’s no need to gobble up your namespace with so many types, especially because it doesn’t enforce anything anyway. The less code you write the better. Here’s the approach I would take:

          enum tiff_entry_type {
             TIFF_ENTRY_SMALL,
             TIFF_ENTRY_BLOCK,
          };
          
          struct tiff_entry {
             enum tiff_entry_type type;
             union {
                uint32_t small;
                struct {
                   const void *ptr;
                   size_t len;
                } block;
             } data;
          };
          
          void
          tiff_entry_set_small(struct tiff_entry *e, uint32_t value)
          {
             e->type = TIFF_ENTRY_SMALL;
             e->data.small = value;
          }
          
          void
          tiff_entry_set_block(struct tiff_entry *e, void *ptr, size_t len)
          {
             e->type = TIFF_ENTRY_BLOCK;
             e->data.block = { .ptr = ptr, .len = len };
          }
          

          I’ll let anybody be the judge on what is more readable and more maintainable.

          1. 1

            Fair point about the suffixes and when to typedef. But your code is about the same as the original code, which the author complained that a TIFF_ENTRY_SMALL had access to TIFF_ENTRY_BLOCK. At least with the code I presented, you can’t mistakenly set small with a TIFF_ENTRY_BLOCK. I also followed the structure return of the original code. I wouldn’t necessarily reject a direct-returned structure, but for me, it depends upon the size of the structure and the context it’s used in.