Compilation Warnings

[BrendonIrwan]

Hello. When compiling with hardened flags:

src/handshake.c: In function ‘strstricase’:
src/handshake.c:90:32: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   90 |                         return (char*)haystack;

To resolve, simply remove the const keyword for haystack.

Another warning lies at toyws.c where tws_receiveframe returns a uint64_t while it is said to return an int.

extra/toyws/toyws.c: In function ‘tws_receiveframe’:
extra/toyws/toyws.c:400:59: warning: operand of ‘?:’ changes signedness from ‘int’ to ‘uint64_t’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Wsign-compare]
  400 |                         return (ret == 0 ? frame_length : ret);
      |                                                           ^~~
extra/toyws/toyws.c:409:43: warning: operand of ‘?:’ changes signedness from ‘int’ to ‘uint64_t’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Wsign-compare]
  409 |         return (ret == 0 ? frame_length : ret);
      |                                           ^~~

For this I dunno how to resolve without breaking. Given that the frame size spans the full unsigned 64-bit range, including zero, we can not allocate a magic value for error.

[BrendonIrwan]

Oh and here are the hardened compilation flags I added in.

# Compiler base and security warning options.
CFLAGS += -g -std=c2x -Wall -Wextra -Wpedantic -Warray-bounds=2 \
		-Wcast-qual -Wformat-overflow=2 -Wformat-truncation=2 -Wformat=2 \
		-Wnull-dereference -Wshift-overflow=2 -Wstack-protector \
		-Wstringop-overflow=4 -Wtrampolines -Wvla
# Compiler hardening options.
CFLAGS += -fPIE -fsanitize-undefined-trap-on-error -fsanitize=undefined \
		-fstack-clash-protection -fstack-protector-strong
# Preprocessor hardening options.
CFLAGS += -D_FORTIFY_SOURCE=3

[Theldus]

Hi @BrendonIrwan,
Thanks for your time reviewing wsServer.

To resolve, simply remove the const keyword for haystack.

Yes, or change the return value to const.

Another warning lies at toyws.c where tws_receiveframe returns a uint64_t while it is said to return an int. For this I dunno how to resolve without breaking. Given that the frame size spans the full unsigned 64-bit range, including zero, we can not allocate a magic value for error.

Oh, this is definitely an issue!.
There's no way to fix this without changing the function signature, so I see two options:

1. Keep int as the return type to signal if there was an error, and add a new parameter uint64_t *frm_size to return the frame size.
2. Change the return type to uint64_t and use it for the frame length. Add a new parameter int *err to signal if there was an error or not.

Both options seems valid to me.

What do you think?

[BrendonIrwan]

If you ask me, then I'd go for option 2.

If I'm not mistaken, a lot of C functions return the actual result while setting any error elsewhere (e.g. errno).

This will require more changes to its dependents though.