fix(makefile)!: improve flags and type casts for stricter build and correctness
- Update Makefile:
- Change test binary name to `libicmp.test`.
- Add `-Wpedantic` and `-Wconversion` to global CFLAGS for stricter
warnings.
- Add dedicated `TEST_CFLAGS`.
- Ensure test build uses `TEST_CFLAGS`.
- Refactor code for better correctness.
BREAKING CHANGE: Test binary is now named `libicmp.test` instead of
`test.out` and stricter compiler flags may trigger new warnings or
errors in code outside this patch.
This commit is contained in:
parent
97a9f24fc3
commit
85543f3d19
6 changed files with 19 additions and 14 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -2,7 +2,7 @@
|
||||||
*.a
|
*.a
|
||||||
*.so
|
*.so
|
||||||
*.so.*
|
*.so.*
|
||||||
test.out
|
libicmp.test
|
||||||
.build/
|
.build/
|
||||||
|
|
||||||
# Object files and dependencies
|
# Object files and dependencies
|
||||||
|
|
|
||||||
7
Makefile
7
Makefile
|
|
@ -3,7 +3,7 @@ LIB_STATIC = lib$(LIB_NAME).a
|
||||||
LIB_SHARED = lib$(LIB_NAME).so
|
LIB_SHARED = lib$(LIB_NAME).so
|
||||||
LIB_VERSION = 1.0.0
|
LIB_VERSION = 1.0.0
|
||||||
LIB_SHARED_VERSIONED = $(LIB_SHARED).$(LIB_VERSION)
|
LIB_SHARED_VERSIONED = $(LIB_SHARED).$(LIB_VERSION)
|
||||||
TEST_BIN = test.out
|
TEST_BIN = libicmp.test
|
||||||
|
|
||||||
# Build configuration
|
# Build configuration
|
||||||
BUILD_STATIC ?= yes
|
BUILD_STATIC ?= yes
|
||||||
|
|
@ -21,10 +21,11 @@ include sources.mk
|
||||||
|
|
||||||
CC = clang
|
CC = clang
|
||||||
CPPFLAGS = -std=c99 -I includes -D_POSIX_C_SOURCE=199309L
|
CPPFLAGS = -std=c99 -I includes -D_POSIX_C_SOURCE=199309L
|
||||||
CFLAGS = -Wall -Wextra -Werror -pipe
|
CFLAGS = -Wall -Wextra -Werror -pipe -Wpedantic -Wconversion
|
||||||
CFLAGS_SHARED = $(CFLAGS) -fPIC
|
CFLAGS_SHARED = $(CFLAGS) -fPIC
|
||||||
LDFLAGS =
|
LDFLAGS =
|
||||||
TEST_LDFLAGS = -lcriterion
|
TEST_LDFLAGS = -lcriterion
|
||||||
|
TEST_CFLAGS = -Wall -Wextra -Werror -pipe
|
||||||
|
|
||||||
OBJ_DIR = .build
|
OBJ_DIR = .build
|
||||||
OBJ_DIR_STATIC = $(OBJ_DIR)/static
|
OBJ_DIR_STATIC = $(OBJ_DIR)/static
|
||||||
|
|
@ -67,7 +68,7 @@ test: $(TEST_BIN)
|
||||||
|
|
||||||
$(TEST_BIN): $(TESTS) $(OBJS_STATIC)
|
$(TEST_BIN): $(TESTS) $(OBJS_STATIC)
|
||||||
@mkdir -p $(dir $@)
|
@mkdir -p $(dir $@)
|
||||||
$(CC) $(CPPFLAGS) -I tests $(CFLAGS) $(TEST_LDFLAGS) $^ -o $@
|
$(CC) $(CPPFLAGS) -I tests $(TEST_CFLAGS) $(TEST_LDFLAGS) $^ -o $@
|
||||||
|
|
||||||
.PHONY: install
|
.PHONY: install
|
||||||
install:
|
install:
|
||||||
|
|
|
||||||
|
|
@ -44,14 +44,17 @@ icmp_parse_ip_header(const void *buffer, size_t buffer_len, uint8_t *ttl,
|
||||||
static int
|
static int
|
||||||
validate_ip_header(const struct ip_header *hdr, size_t buffer_len)
|
validate_ip_header(const struct ip_header *hdr, size_t buffer_len)
|
||||||
{
|
{
|
||||||
|
uint8_t ihl;
|
||||||
|
size_t header_len;
|
||||||
|
|
||||||
if (4 != (hdr->version_ihl >> 4))
|
if (4 != (hdr->version_ihl >> 4))
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
uint8_t ihl = hdr->version_ihl & 0x0F;
|
ihl = hdr->version_ihl & 0x0F;
|
||||||
if (ihl < 5)
|
if (ihl < 5)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
size_t header_len = ihl << 2;
|
header_len = (size_t)(ihl << 2);
|
||||||
if (header_len > buffer_len)
|
if (header_len > buffer_len)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
|
|
@ -61,5 +64,5 @@ validate_ip_header(const struct ip_header *hdr, size_t buffer_len)
|
||||||
static size_t
|
static size_t
|
||||||
extract_ip_header_length(uint8_t version_ihl)
|
extract_ip_header_length(uint8_t version_ihl)
|
||||||
{
|
{
|
||||||
return (version_ihl & 0x0F) << 2;
|
return (size_t)((version_ihl & 0x0F) << 2);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
#include "internal/icmp_recv.h"
|
#include "internal/icmp_recv.h"
|
||||||
|
|
||||||
|
// TODO: check ssize_t -> size_t cast
|
||||||
void
|
void
|
||||||
recv_process_single_packet(uint8_t *buffer, ssize_t len, icmp_callback_t cb,
|
recv_process_single_packet(uint8_t *buffer, ssize_t len, icmp_callback_t cb,
|
||||||
void *userdata)
|
void *userdata)
|
||||||
|
|
@ -10,11 +11,11 @@ recv_process_single_packet(uint8_t *buffer, ssize_t len, icmp_callback_t cb,
|
||||||
const void *payload;
|
const void *payload;
|
||||||
size_t payload_len, ip_hdr_len;
|
size_t payload_len, ip_hdr_len;
|
||||||
|
|
||||||
if (recv_parse_packet(buffer, len, &type, &code, &ttl, &src_addr,
|
if (recv_parse_packet(buffer, (size_t)(len), &type, &code, &ttl, &src_addr,
|
||||||
&payload, &payload_len, &ip_hdr_len) < 0)
|
&payload, &payload_len, &ip_hdr_len) < 0)
|
||||||
return;
|
return;
|
||||||
recv_build_reply(&reply, type, code, ttl, src_addr, payload, payload_len,
|
recv_build_reply(&reply, type, code, ttl, src_addr, payload, payload_len,
|
||||||
buffer, ip_hdr_len);
|
buffer, ip_hdr_len);
|
||||||
reply.ip_payload_len = len - ip_hdr_len;
|
reply.ip_payload_len = (size_t)(len) - ip_hdr_len;
|
||||||
cb(&reply, userdata);
|
cb(&reply, userdata);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -36,7 +36,7 @@ icmp_send_raw(icmp_handle_t *h, uint8_t type, uint8_t code,
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
return send_to_destination(h, buffer, packet_len, dest, ttl);
|
return send_to_destination(h, buffer, (size_t)(packet_len), dest, ttl);
|
||||||
}
|
}
|
||||||
|
|
||||||
static int
|
static int
|
||||||
|
|
|
||||||
|
|
@ -16,7 +16,7 @@ icmp_checksum(const void *data, size_t len)
|
||||||
sum = sum_words(data, len);
|
sum = sum_words(data, len);
|
||||||
sum = fold_carries(sum);
|
sum = fold_carries(sum);
|
||||||
|
|
||||||
return ~sum;
|
return (uint16_t)~sum;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Sum all 16-bit words */
|
/* Sum all 16-bit words */
|
||||||
|
|
@ -42,7 +42,7 @@ sum_words(const uint8_t *data, size_t len)
|
||||||
|
|
||||||
/* Handle odd byte if present */
|
/* Handle odd byte if present */
|
||||||
if (len & 1)
|
if (len & 1)
|
||||||
sum += *ptr << 8;
|
sum += (uint32_t)(*ptr << 8);
|
||||||
|
|
||||||
return sum;
|
return sum;
|
||||||
}
|
}
|
||||||
|
|
@ -53,12 +53,12 @@ fold_carries(uint32_t sum)
|
||||||
{
|
{
|
||||||
while (sum >> 16)
|
while (sum >> 16)
|
||||||
sum = (sum & 0xFFFF) + (sum >> 16);
|
sum = (sum & 0xFFFF) + (sum >> 16);
|
||||||
return sum;
|
return (uint16_t)sum;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Read 16-bit word in network byte order (big-endian) */
|
/* Read 16-bit word in network byte order (big-endian) */
|
||||||
static inline uint16_t
|
static inline uint16_t
|
||||||
read_be16(const uint8_t *ptr)
|
read_be16(const uint8_t *ptr)
|
||||||
{
|
{
|
||||||
return (*ptr << 8) | ptr[1];
|
return (uint16_t)((*ptr << 8) | ptr[1]);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue