Skip to content

Commit a1fd731

Browse files
feat: add GCC/Clang function attributes for compile-time bug detection
Add SPINE_ATTR_* compatibility macros and apply them across all header files for zero-runtime-cost compile-time checking: - SPINE_ATTR_FORMAT: printf format string validation on spine_log, die - SPINE_ATTR_NORETURN: on die() for dead code detection - SPINE_ATTR_WARN_UNUSED: on db_query, exec_poll, php_cmd, snmp_get* to catch ignored return values and memory leaks - SPINE_ATTR_NONNULL: on functions requiring non-NULL arguments (db_query, db_insert, strncopy, snmp_get*, etc.) - SPINE_ATTR_PURE: on side-effect-free functions (is_numeric, is_hexadecimal, validate_result, keyword parsers) - SPINE_ATTR_COLD: on die() for branch prediction optimization Bugs found and fixed by warn_unused_result: - poller.c: 3 db_query() calls for DML (UPDATE/INSERT) ignored the return value, leaking MYSQL_RES. Wrapped with db_free_result(). Applied to 30+ function declarations across 8 header files. Gracefully compiles as no-ops on non-GCC/Clang compilers. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent 2b14ecd commit a1fd731

8 files changed

Lines changed: 95 additions & 37 deletions

File tree

keywords.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,18 @@
3131
+-------------------------------------------------------------------------+
3232
*/
3333

34-
extern const char *printable_log_level(int token);
35-
extern int parse_log_level(const char *word, int dflt);
34+
extern const char *printable_log_level(int token)
35+
SPINE_ATTR_PURE;
36+
extern int parse_log_level(const char *word, int dflt)
37+
SPINE_ATTR_PURE;
3638

37-
extern const char *printable_logdest(int token);
38-
extern int parse_logdest(const char *word, int dflt);
39+
extern const char *printable_logdest(int token)
40+
SPINE_ATTR_PURE;
41+
extern int parse_logdest(const char *word, int dflt)
42+
SPINE_ATTR_PURE;
3943

40-
extern const char *printable_action(int token);
41-
extern int parse_action(const char *word, int dflt);
44+
extern const char *printable_action(int token)
45+
SPINE_ATTR_PURE;
46+
extern int parse_action(const char *word, int dflt)
47+
SPINE_ATTR_PURE;
4248

php.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@
3131
+-------------------------------------------------------------------------+
3232
*/
3333

34-
extern char *php_cmd(const char *php_command, int php_process);
35-
extern char *php_readpipe(int php_process, char *command);
34+
extern char *php_cmd(const char *php_command, int php_process)
35+
SPINE_ATTR_NONNULL(1)
36+
SPINE_ATTR_WARN_UNUSED;
37+
extern char *php_readpipe(int php_process, char *command)
38+
SPINE_ATTR_WARN_UNUSED;
3639
extern int php_init(int php_process);
3740
extern void php_close(int php_process);
3841
extern int php_get_process(void);

poller.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2003,7 +2003,7 @@ void poll_host(int device_counter, int host_id, int host_thread, int host_thread
20032003
if (host_thread == host_threads && set.active_profiles != 1) {
20042004
SPINE_LOG_MEDIUM(("Device[%i] HT[%i] Updating Poller Items for Next Poll", host_id, host_thread));
20052005

2006-
db_query(&mysql, LOCAL, query6);
2006+
db_free_result(db_query(&mysql, LOCAL, query6));
20072007
}
20082008

20092009
/* record the polling time for the device */
@@ -2023,7 +2023,7 @@ void poll_host(int device_counter, int host_id, int host_thread, int host_thread
20232023
poll_time = get_time_as_double();
20242024
query1[0] = '\0';
20252025
snprintf(query1, BUFSIZE, "UPDATE host SET polling_time = %.3f - %.3f WHERE id = %i", poll_time, host_time_double, host_id);
2026-
db_query(&mysql, LOCAL, query1);
2026+
db_free_result(db_query(&mysql, LOCAL, query1));
20272027

20282028
}
20292029

@@ -2041,7 +2041,7 @@ void poll_host(int device_counter, int host_id, int host_thread, int host_thread
20412041
" local_data_ids = CONCAT(local_data_ids, \", \", VALUES(local_data_ids))",
20422042
host_id, set.poller_id, errors, error_string);
20432043

2044-
db_query(&mysql, LOCAL, error_query);
2044+
db_free_result(db_query(&mysql, LOCAL, error_query));
20452045

20462046
free(error_query);
20472047
}

poller.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,13 @@ extern void child_cleanup(void *arg);
3636
extern void child_cleanup_thread(void *arg);
3737
extern void child_cleanup_script(void *arg);
3838
extern void poll_host(int device_counter, int host_id, int host_thread, int host_threads, int host_data_ids, char *host_time, int *host_errors, double host_time_double);
39-
extern char *exec_poll(host_t *current_host, char *command, int id, const char *type);
40-
extern void get_system_information(host_t *host, MYSQL *mysql, int system);
41-
extern int is_multipart_output(char *result);
42-
extern int validate_result(char *result);
39+
extern char *exec_poll(host_t *current_host, char *command, int id, const char *type)
40+
SPINE_ATTR_NONNULL(1, 2)
41+
SPINE_ATTR_WARN_UNUSED;
42+
extern void get_system_information(host_t *host, MYSQL *mysql, int system)
43+
SPINE_ATTR_NONNULL(1, 2);
44+
extern int is_multipart_output(char *result)
45+
SPINE_ATTR_PURE;
46+
extern int validate_result(char *result)
47+
SPINE_ATTR_PURE;
4348
extern void buffer_output_errors(char * error_string, int * buf_size, int * buf_errors, int device_id, int thread_id, int local_data_id, bool flush);

snmp.h

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,17 @@ extern void snmp_spine_init(void);
3737
extern void snmp_spine_close(void);
3838
extern void *snmp_host_init(int host_id, char *hostname, int snmp_version, char *snmp_community, char *snmp_username, char *snmp_password, char *snmp_auth_protocol, char *snmp_priv_passphrase, char *snmp_priv_protocol, char *snmp_context, char *snmp_engine_id, int snmp_port, int snmp_timeout);
3939
extern void snmp_host_cleanup(void *snmp_session);
40-
extern char *snmp_get_base(host_t *current_host, const char *snmp_oid, bool should_fail);
41-
extern char *snmp_get(host_t *current_host, const char *snmp_oid);
42-
extern char *snmp_getnext(host_t *current_host, const char *snmp_oid);
43-
extern int snmp_count(host_t *current_host, const char *snmp_oid);
44-
extern void snmp_get_multi(host_t *current_host, target_t *poller_items, snmp_oids_t *snmp_oids, int num_oids);
40+
extern char *snmp_get_base(host_t *current_host, const char *snmp_oid, bool should_fail)
41+
SPINE_ATTR_NONNULL(1, 2)
42+
SPINE_ATTR_WARN_UNUSED;
43+
extern char *snmp_get(host_t *current_host, const char *snmp_oid)
44+
SPINE_ATTR_NONNULL(1, 2)
45+
SPINE_ATTR_WARN_UNUSED;
46+
extern char *snmp_getnext(host_t *current_host, const char *snmp_oid)
47+
SPINE_ATTR_NONNULL(1, 2)
48+
SPINE_ATTR_WARN_UNUSED;
49+
extern int snmp_count(host_t *current_host, const char *snmp_oid)
50+
SPINE_ATTR_NONNULL(1, 2);
51+
extern void snmp_get_multi(host_t *current_host, target_t *poller_items, snmp_oids_t *snmp_oids, int num_oids)
52+
SPINE_ATTR_NONNULL(1, 2, 3);
4553
extern void snmp_snprint_value(char *obuf, size_t buf_len, const oid *objid, size_t objidlen, struct variable_list *variable);

spine.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,26 @@
5353
# define __attribute__(x) /* NOTHING */
5454
#endif
5555

56+
/* Function attribute macros for GCC/Clang compile-time checks.
57+
* These expand to nothing on non-GCC/Clang compilers.
58+
*/
59+
#ifdef __GNUC__
60+
#define SPINE_ATTR_FORMAT(archetype, string_index, first_to_check) \
61+
__attribute__((format(archetype, string_index, first_to_check)))
62+
#define SPINE_ATTR_NORETURN __attribute__((noreturn))
63+
#define SPINE_ATTR_WARN_UNUSED __attribute__((warn_unused_result))
64+
#define SPINE_ATTR_NONNULL(...) __attribute__((nonnull(__VA_ARGS__)))
65+
#define SPINE_ATTR_PURE __attribute__((pure))
66+
#define SPINE_ATTR_COLD __attribute__((cold))
67+
#else
68+
#define SPINE_ATTR_FORMAT(archetype, string_index, first_to_check)
69+
#define SPINE_ATTR_NORETURN
70+
#define SPINE_ATTR_WARN_UNUSED
71+
#define SPINE_ATTR_NONNULL(...)
72+
#define SPINE_ATTR_PURE
73+
#define SPINE_ATTR_COLD
74+
#endif
75+
5676
/* Windows does not support stderr. Therefore, don't use it. */
5777
#ifdef __CYGWIN__
5878
#define DISABLE_STDERR

sql.h

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,30 @@
3131
+-------------------------------------------------------------------------+
3232
*/
3333

34-
extern int db_insert(MYSQL *mysql, int type, const char *query);
35-
extern MYSQL_RES *db_query(MYSQL *mysql, int type, const char *query);
36-
extern void db_connect(int type, MYSQL *mysql);
37-
extern void db_disconnect(MYSQL *mysql);
38-
extern void db_escape(MYSQL *mysql, char *output, int max_size, const char *input);
34+
extern int db_insert(MYSQL *mysql, int type, const char *query)
35+
SPINE_ATTR_NONNULL(1, 3);
36+
extern MYSQL_RES *db_query(MYSQL *mysql, int type, const char *query)
37+
SPINE_ATTR_NONNULL(1, 3)
38+
SPINE_ATTR_WARN_UNUSED;
39+
extern void db_connect(int type, MYSQL *mysql)
40+
SPINE_ATTR_NONNULL(2);
41+
extern void db_disconnect(MYSQL *mysql)
42+
SPINE_ATTR_NONNULL(1);
43+
extern void db_escape(MYSQL *mysql, char *output, int max_size, const char *input)
44+
SPINE_ATTR_NONNULL(1, 2, 4);
3945
extern void db_free_result(MYSQL_RES *result);
4046
extern void db_create_connection_pool(int type);
4147
extern void db_close_connection_pool(int type);
42-
extern pool_t *db_get_connection(int type);
48+
extern pool_t *db_get_connection(int type)
49+
SPINE_ATTR_WARN_UNUSED;
4350
extern void db_release_connection(int type, int id);
44-
extern int db_reconnect(MYSQL *mysql, int type, int error, const char *location);
45-
extern int db_column_exists(MYSQL *mysql, int type, const char *table, const char *column);
51+
extern int db_reconnect(MYSQL *mysql, int type, int error, const char *location)
52+
SPINE_ATTR_NONNULL(1);
53+
extern int db_column_exists(MYSQL *mysql, int type, const char *table, const char *column)
54+
SPINE_ATTR_NONNULL(1, 3, 4);
4655

47-
extern int append_hostrange(char *obuf, const char *colname);
56+
extern int append_hostrange(char *obuf, const char *colname)
57+
SPINE_ATTR_NONNULL(1, 2);
4858

4959
#define MYSQL_SET_OPTION(opt, value, desc) \
5060
{\

util.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,25 @@ extern void config_defaults(void);
3838

3939
/* cacti logging function */
4040
extern int spine_log(const char *format, ...)
41-
__attribute__((format(printf, 1, 2)));
41+
SPINE_ATTR_FORMAT(printf, 1, 2);
4242

4343
extern void die(const char *format, ...)
44-
__attribute__((noreturn))
45-
__attribute__((format(printf, 1, 2)));
44+
SPINE_ATTR_NORETURN
45+
SPINE_ATTR_COLD
46+
SPINE_ATTR_FORMAT(printf, 1, 2);
4647

4748
/* option processing function */
4849
extern void set_option(const char *setting, const char *value);
4950

5051
/* number validation functions */
51-
extern int is_numeric(char *string);
52-
extern int is_ipaddress(const char *string);
53-
extern int all_digits(const char *str);
54-
extern int is_hexadecimal(const char * str, const short ignore_special);
52+
extern int is_numeric(char *string)
53+
SPINE_ATTR_PURE;
54+
extern int is_ipaddress(const char *string)
55+
SPINE_ATTR_PURE;
56+
extern int all_digits(const char *str)
57+
SPINE_ATTR_PURE;
58+
extern int is_hexadecimal(const char * str, const short ignore_special)
59+
SPINE_ATTR_PURE;
5560

5661
/* determine if a device is a debug device */
5762
extern int is_debug_device(int device_id);
@@ -60,7 +65,8 @@ extern int is_debug_device(int device_id);
6065
extern char *add_slashes(char *string);
6166
extern int file_exists(const char *filename);
6267
extern char *strip_alpha(char *string);
63-
extern char *strncopy(char *dst, const char *src, size_t n);
68+
extern char *strncopy(char *dst, const char *src, size_t n)
69+
SPINE_ATTR_NONNULL(1, 2);
6470
extern char *trim(char *str);
6571
extern char *rtrim(char *str);
6672
extern char *ltrim(char *str);

0 commit comments

Comments
 (0)