server icon indicating copy to clipboard operation
server copied to clipboard

MDEV-34348: Enable -Wcast-function-type-strict

Open bnestere opened this issue 1 year ago • 2 comments
trafficstars

Originally, this PR was just a preliminary patch for one specific MDEV-34348 change for early feedback. Now, it contains the remainder of the patch. For now, this is only done in the scope of 10.5. Before merging, I'm going to merge through up to 11.7 to see if there are any changes that would be better done here. The current 10.5 version should be good to review though, and I imagine other changes will be minor.

Original description of the PR for reference:

A large contributor to -Wcast-function-type-strict errors comes from qsort2_cmp and queue_compare functions often cast as one-another. Each function’s signature pre-patch is:

typedef int (*qsort2_cmp)(const void *, const void *, const void *);

typedef int (*queue_compare)(void *,uchar *, uchar *);

This patch proposes changing the queue_compare signature to match that of qsort2_cmp, which is more strict with const parameters. I couldn’t find an implementation of queue_compare which violates the const semantics.

Other alternatives I considered were:

  1. For each case where a function is used as qsort2_cmp and queue_compare, define two functions, one for each signature. This seems heavy, and also leads to additional complications, e.g. for the function get_compare_function:

qsort2_cmp get_compare_function() const { return using_packed_sortkeys() ? get_packed_keys_compare_ptr() : get_ptr_compare(sort_length); }

2.Add some additional logic for queues.h interface to allow a qsort2_cmp comparator (with a different signature), though I couldn’t think of a way to do it without additional conditional overhead.

  1. Make these findings exceptions to the flag

  2. Switch qsort2_cmp to use queue_compare's signature, though I think const makes more sense to have, as compare should be a read-only operation.

bnestere avatar Aug 30 '24 16:08 bnestere