Page MenuHomeFreeBSD

include: Implement N2867.
ClosedPublic

Authored by des on Sep 5 2023, 3:41 PM.
Tags
None
Referenced Files
F102202107: D41734.diff
Fri, Nov 8, 9:49 PM
Unknown Object (File)
Mon, Oct 21, 3:51 PM
Unknown Object (File)
Fri, Oct 11, 3:45 AM
Unknown Object (File)
Sep 27 2024, 7:53 AM
Unknown Object (File)
Sep 27 2024, 7:53 AM
Unknown Object (File)
Sep 27 2024, 7:53 AM
Unknown Object (File)
Sep 27 2024, 7:53 AM
Unknown Object (File)
Sep 27 2024, 7:53 AM
Subscribers

Details

Summary

This adds macros for checked addition, subtraction, and multiplication with semantics similar to the builtins gcc and clang have had for years.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

des requested review of this revision.Sep 5 2023, 3:41 PM
des retitled this revision from libc: Implement N2867. to include: Implement N2867..Sep 5 2023, 4:08 PM
kib added inline comments.
include/stdckdint.h
8

These names pollute user namespace. Typically we prepend the underscore to upper-case symbols to avoid that.

include/stdckdint.h
8

I was considering just using __STDC_VERSION_STDCKDINT_H__ here, can you see any reason not to?

Use the feature macro as include guard.

include/stdckdint.h
8

I forget: can we use #pragma once yet?

8

Though this is a good way to do it too

des marked 2 inline comments as done.Sep 6 2023, 3:35 AM
des added inline comments.
include/stdckdint.h
8

I forget: can we use #pragma once yet?

Rejected by WG14 because it was deemed too hard to describe portably. All the compilers we care about support it, though, so we could just decide to use it regardless. I wouldn't be surprised if it speeds up the build noticeably.

des marked an inline comment as done.Sep 6 2023, 4:48 AM
include/stdckdint.h
20

Isn't __has_builtin clang-specific? Did you tested this with gcc? I suspect that gcc would always hit _Static_assert()s despite providing the built-ins.

include/stdckdint.h
20

(recent enough) GCC supports __has_builtin
https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fbuiltin.html

In 20bd59416dca (when an earlier GCC version was relevant) I used:

#if __GNUC__ >= 5 || \
    (defined(__has_builtin) && __has_builtin(__builtin_add_overflow))
        if (__builtin_add_overflow(a, b, &result))
                errx(1, "Corrupt patch");
#else
        if ((b > 0 && a > OFF_MAX - b) || (b < 0 && a < OFF_MIN - b))
                errx(1, "Corrupt patch");
        result = a + b;
#endif
des marked 2 inline comments as done.

support gcc < 10

include/stdckdint.h
20

GCC has had __has_builtin() since 10.1, but I'm not 100% satisfied that we no longer care about GCC 9. I've regenerated the patch.

This revision is now accepted and ready to land.Sep 6 2023, 3:08 PM

similar to my comment in the other review would be nice to briefly mention what N2867 is in the commit subject like include: Implement N2867, checked addition subtraction and multiplication or overflow-checked math or so

similar to my comment in the other review would be nice to briefly mention what N2867 is in the commit subject

image.png (65×809 px, 7 KB)

This revision was automatically updated to reflect the committed changes.