Server fails to respond on IPP requests due to enum size assumption.

[SebastiaanMerckx]

Describe the bug
I've been porting the application to a microcontroller using zephyr. It receives an IPP request, but the received operation_id is 0. Eventually, it seems that the size of enums like ipp_op_t and ipp_status_t (in typedef union _ipp_request_u is not necessarily the size of an int (op_status).
Therefore, the different union members do not necessarily align. In our case, the size of an enum was 2 bytes, size of int 4 bytes.

To Reproduce
Steps to reproduce the behavior:

1. Compile with a compiler that wraps enums in 2 bytes. Verify that the union structs do not align when receiving an IPP packet. _ipp_request_u.any.op_status != _ipp_request_u.op.operation_id.

Possible solutions
I'm not sure what's preferred...

• I forced the enums to be at least "int" size by adding an enum value of 0xFFFFFFFF but evidently that only works on 32-bit systems.
• Alternatively, the enums in the struct could be replaced with ints.
• But these solutions probably don't ensure correct behavior. Maybe the structs even need to be packed to make sure that the variable align?

[michaelrsweet]

This structure/union combo is basically unchanged from 1997 and we have not had reports of issues across hundreds of architectures, OS's, and compilers. The only assumption we've made over the years is that the CPU architecture provides at least 32-bit ints and that your system has at least low megabytes of memory available (the original implementation was on a MIPS R2000A SGI system with 8MB of RAM...)

What compiler are you using? What architecture is the controller you are targeting? What compiler options are you using?

[SebastiaanMerckx]

The target architecture is cortex-M33.
Compiler is arm-zephyr-eabi-gcc.exe (Zephyr SDK 0.17.4) 12.2.0
Compiler options: we use plenty, but even the simplest file compiles with variable enum lengths.

Example: size of status is 1, 2 or 4 bytes depending on whether you include TMP2 and TMP3.

typedef enum ipp_status_e
{
	TMP0 = 0,
	TMP1 = 1,
	TMP2 = 0xffff,
	//TMP3 = 0xffffffff
} ipp_status_t;

volatile ipp_status_t status = TMP1;

int main() {
	return status;
}

$ arm-zephyr-eabi-gcc.exe -mcpu=cortex-m33+nodsp -fdata-sections  -c main.c
$ objdump -x main.o

main.o:     file format elf32-little
main.o
architecture: UNKNOWN!, flags 0x00000011:
HAS_RELOC, HAS_SYMS
start address 0x00000000

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00000018  00000000  00000000  00000034  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
  1 .data         00000000  00000000  00000000  0000004c  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  2 .bss          00000000  00000000  00000000  0000004c  2**0
                  ALLOC
  3 .data.status  00000002  00000000  00000000  0000004c  2**1
                  CONTENTS, ALLOC, LOAD, DATA
  4 .comment      00000021  00000000  00000000  0000004e  2**0
                  CONTENTS, READONLY
  5 .ARM.attributes 00000032  00000000  00000000  0000006f  2**0
                  CONTENTS, READONLY
SYMBOL TABLE:
00000000 l    df *ABS*  00000000 main.c
00000000 l    d  .text  00000000 .text
00000000 l    d  .data  00000000 .data
00000000 l    d  .bss   00000000 .bss
00000000 l    d  .data.status   00000000 .data.status
00000000 l       .data.status   00000000 $d
00000000 l       .text  00000000 $t
00000014 l       .text  00000000 $d
00000000 l    d  .comment       00000000 .comment
00000000 l    d  .ARM.attributes        00000000 .ARM.attributes
00000000 g     O .data.status   00000002 status
00000001 g     F .text  00000018 main


RELOCATION RECORDS FOR [.text]:
OFFSET   TYPE              VALUE
00000014 UNKNOWN           status

I'll search for reasons why the zephyr compiler has this default behavior.

[SebastiaanMerckx]

Ok, apparently the option -fno-short-enums should work for us too. The zephyr compiler is heavily optimized for RAM reduction and the C standard perfectly allows variable-sized enums.

[michaelrsweet]

Well, C89 and C99 tie enums to "int" or "unsigned int". C11 and C23 change that, with C23 allowing you to specify the underlying integer type.

You'll have trouble compiling with "-fno-short-enums" since it affects all enums, not just the CUPS code. I'll see what I can come up with that is portable...

[michaelrsweet]

OK, I think what I'm going to do is refactor the (private) _ipp_request_t union to be a struct, and cast the op_status value accordingly in the ippGetOperation and ippGetStatusCode functions. The op_status value can also just be a short since that is what the value is on the wire anyways...

I'll make a similar change to the CUPS 2.5 code base as well.