| Message ID | 20240608183747.2084294-1-gitster@pobox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | varargs functions with __attributes__(()) | expand |
On Sat, Jun 08, 2024 at 11:37:43AM -0700, Junio C Hamano wrote: > There are several varargs functions that take either NULL-terminated > list of parameters, or printf-like format followed by list of > parameters, that are not declared as such with __attributes__(()). > > Adding such a missing attribute to trace2_region_enter_printf() > revealed that an existing call to it was trying to format a value of > type size_t using "%d", which is not such an excellent idea. Other > functions that were lacking attributes fortunately did not have any > broken existing callers. Great, I am happy to see these. I assume you found them all by grepping? I wonder if there is a way to convince the compiler (or coccinelle) to complain about any varargs function that does not have one of our usual annotations. It's possible to have other conventions (e.g., an "int" up front specifying the number of entries) but in practice I doubt we would ever use one. Still, I suspect the answer is probably "no", there is not an easy way to do it. -Peff
Jeff King <peff@peff.net> writes: > On Sat, Jun 08, 2024 at 11:37:43AM -0700, Junio C Hamano wrote: > >> There are several varargs functions that take either NULL-terminated >> list of parameters, or printf-like format followed by list of >> parameters, that are not declared as such with __attributes__(()). >> >> Adding such a missing attribute to trace2_region_enter_printf() >> revealed that an existing call to it was trying to format a value of >> type size_t using "%d", which is not such an excellent idea. Other >> functions that were lacking attributes fortunately did not have any >> broken existing callers. > > Great, I am happy to see these. I assume you found them all by grepping? > > I wonder if there is a way to convince the compiler (or coccinelle) to > complain about any varargs function that does not have one of our > usual annotations. It's possible to have other conventions (e.g., an > "int" up front specifying the number of entries) but in practice I doubt > we would ever use one. > > Still, I suspect the answer is probably "no", there is not an easy way > to do it. I just went from grepping for fixed "...)" in *.[ch] files. I do admit I wished some form of automation, but didn't come up with one. I was happy that the exercise found a real bug ;-) I started it only as a clean-up topic.
There are several varargs functions that take either NULL-terminated list of parameters, or printf-like format followed by list of parameters, that are not declared as such with __attributes__(()). Adding such a missing attribute to trace2_region_enter_printf() revealed that an existing call to it was trying to format a value of type size_t using "%d", which is not such an excellent idea. Other functions that were lacking attributes fortunately did not have any broken existing callers. Junio C Hamano (4): __attribute__: trace2_region_enter_printf() is like "printf" __attribute__: remove redundant __attribute__ declaration for git_die_config() __attribute__: mark some functions with LAST_ARG_MUST_BE_NULL __attribute__: add a few missing format attributes add-patch.c | 1 + attr.h | 2 ++ config.c | 1 - hook.h | 1 + mem-pool.h | 1 + run-command.c | 3 ++- scalar.c | 2 ++ trace2.h | 1 + wt-status.c | 1 + 9 files changed, 11 insertions(+), 2 deletions(-)