On Mon, Jul 6, 2026 at 1:16 PM Eelco Chaudron <[email protected]> wrote: > > > > On 2 Jul 2026, at 21:59, Dmitry Mityugov wrote: > > > On Wed, Jun 24, 2026 at 2:48 PM Eelco Chaudron <[email protected]> wrote: > >> > >> > >> > >> On 14 Jun 2026, at 13:20, Dmitry Mityugov wrote: > >> > >>> When included in C++ code, header file lib/packets.h delivers the > >>> following warning when compiled with GCC (16.1 and earlier versions): > >>> > >>> warning: invalid suffix on literal; C++11 requires a space between > >>> literal and string macro [-Wliteral-suffix] > >>> > >>> To reproduce this problem, its enough to run this command from the root > >>> of Open vSwitch repository: > >>> > >>> g++ lib/packets.h -Iinclude > >>> > >>> This patch fixes this by adding necessary spaces. It also disables C++ > >>> name mangling for the C code by conditionally adding extern "C" > >>> directive. > >>> > >>> Signed-off-by: Dmitry Mityugov <[email protected]> > >> > >> Hi Dmitry, > >> > >> Thanks for the series, see a comment below. > >> > >> //Eelco > >> > >>> --- > >>> lib/packets.h | 27 +++++++++++++++++++-------- > >>> 1 file changed, 19 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/lib/packets.h b/lib/packets.h > >>> index ea0bda840..38318b771 100644 > >>> --- a/lib/packets.h > >>> +++ b/lib/packets.h > >>> @@ -34,6 +34,10 @@ > >>> #include "util.h" > >>> #include "timeval.h" > >>> > >>> +#ifdef __cplusplus > >>> +extern "C" { > >>> +#endif > >>> + > >>> struct dp_packet; > >>> struct conn; > >>> struct ds; > >>> @@ -367,7 +371,8 @@ void add_mpls(struct dp_packet *packet, ovs_be16 > >>> ethtype, ovs_be32 lse, > >>> * > >>> */ > >>> #define ETH_ADDR_FMT \ > >>> - "%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8 > >>> + "%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 ":" \ > >>> + "%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 > >> > >> The coding style guide document states that we should not use > >> spaces here. Not sure if we still have antique printf() > >> implementations, however, could you not compile with > >> -Wno-literal-suffix? We could also do something with a > >> 'clang/GCC diagnostic ignored' in the #ifdef, but not sure if > >> it's worth the overhead from an OVS perspective. > >> > >> Ilya any thoughts on this? > > > > Hi Eelco, > > > > Thank you for your attention to this minor problem and for sharing the > > workaround. I re-read the style guide and found no word on using > > spaces in this case, although the examples provided don't use them. > > It's my understanding that the "antique printf() implementations" that > > the documentation refers to don't understand "%zu" and other modern > > format specifiers, not the extra space (that would be parsed out by > > the compiler long before the string reaches printf() anyway). The > > string concatenation (that is, merging strings like these "Hello " > > "World" into one) didn't exist in original K&R and was introduced by > > the ANSI 89 C standard, but compiling my patch with -std=c89 produces > > no errors other than expected "unknown inline keyword" and "variable > > declarations inside loops". In addition, indent with the options > > suggested in the style guide is also silent about my changes. > > > > So it would be nice, if time permits, to clarify if extra spaces can > > be used in such cases in the style guide, preferably with brief > > references to the compilers that don't handle them. Otherwise, I'm > > happy with the workaround above, thank you again. > > Hi Dmitry, > > Thank you for looking into this. However, we will not accept the format > with spaces, as this creates an inconsistent style compared to the > existing code in the codebase. > > Please send a new revision without these formatting changes. The preferred > approach would be to fix this in the project that includes this header file, > rather than changing the OVS header itself.
Hi Eelco, The whole idea of this patch was to add the spaces to make OVS code compatible with C++ 11, so it's pointless to resend the patch without the spaces. Let this patch go the the great collection of the unfinished patches in the Sky. It's my understanding that OVS is a pure C project so making it compatible with C++ is not in great priority; I understand that. Making it compatible with C++ requires much more effort that just modifying lib/packets.h. I'll be glad to resume this activity when a more appropriate time comes. Best regards, > //Eelco > > >>> #define ETH_ADDR_ARGS(EA) ETH_ADDR_BYTES_ARGS((EA).ea) > >>> #define ETH_ADDR_BYTES_ARGS(EAB) \ > >>> (EAB)[0], (EAB)[1], (EAB)[2], (EAB)[3], (EAB)[4], (EAB)[5] > >>> @@ -381,8 +386,8 @@ void add_mpls(struct dp_packet *packet, ovs_be16 > >>> ethtype, ovs_be32 lse, > >>> * > >>> */ > >>> #define ETH_ADDR64_FMT \ > >>> - "%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8":" \ > >>> - "%02"PRIx8":%02"PRIx8":%02"PRIx8":%02"PRIx8 > >>> + "%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 ":" \ > >>> + "%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 ":%02" PRIx8 > >>> #define ETH_ADDR64_ARGS(EA) ETH_ADDR64_BYTES_ARGS((EA).ea64) > >>> #define ETH_ADDR64_BYTES_ARGS(EAB) \ > >>> (EAB)[0], (EAB)[1], (EAB)[2], (EAB)[3], \ > >>> @@ -400,7 +405,8 @@ void add_mpls(struct dp_packet *packet, ovs_be16 > >>> ethtype, ovs_be32 lse, > >>> * ... > >>> * } > >>> */ > >>> -#define ETH_ADDR_SCAN_FMT > >>> "%"SCNx8":%"SCNx8":%"SCNx8":%"SCNx8":%"SCNx8":%"SCNx8 > >>> +#define ETH_ADDR_SCAN_FMT "%" SCNx8 ":%" SCNx8 ":%" SCNx8 ":%" SCNx8 ":" > >>> \ > >>> + "%" SCNx8 ":%" SCNx8 > >>> #define ETH_ADDR_SCAN_ARGS(EA) \ > >>> &(EA).ea[0], &(EA).ea[1], &(EA).ea[2], &(EA).ea[3], &(EA).ea[4], > >>> &(EA).ea[5] > >>> > >>> @@ -625,7 +631,7 @@ mpls_lse_to_bos(ovs_be32 mpls_lse) > >>> return (mpls_lse & htonl(MPLS_BOS_MASK)) != 0; > >>> } > >>> > >>> -#define IP_FMT "%"PRIu32".%"PRIu32".%"PRIu32".%"PRIu32 > >>> +#define IP_FMT "%" PRIu32 ".%" PRIu32 ".%" PRIu32 ".%" PRIu32 > >>> #define IP_ARGS(ip) \ > >>> ntohl(ip) >> 24, \ > >>> (ntohl(ip) >> 16) & 0xff, \ > >>> @@ -642,15 +648,16 @@ mpls_lse_to_bos(ovs_be32 mpls_lse) > >>> * ... > >>> * } > >>> */ > >>> -#define IP_SCAN_FMT "%"SCNu8".%"SCNu8".%"SCNu8".%"SCNu8 > >>> +#define IP_SCAN_FMT "%" SCNu8 ".%" SCNu8 ".%" SCNu8 ".%" SCNu8 > >>> #define IP_SCAN_ARGS(ip) \ > >>> ((void) (ovs_be32) *(ip), &((uint8_t *) ip)[0]), \ > >>> &((uint8_t *) ip)[1], \ > >>> &((uint8_t *) ip)[2], \ > >>> &((uint8_t *) ip)[3] > >>> > >>> -#define IP_PORT_SCAN_FMT "%"SCNu8".%"SCNu8".%"SCNu8".%"SCNu8":%"SCNu16 > >>> -#define IP_PORT_SCAN_ARGS(ip, port) \ > >>> +#define IP_PORT_SCAN_FMT "%" SCNu8 ".%" SCNu8 ".%" SCNu8 ".%" SCNu8 \ > >>> + ":%" SCNu16 > >>> +#define IP_PORT_SCAN_ARGS(ip, port) \ > >>> ((void) (ovs_be32) *(ip), &((uint8_t *) ip)[0]), \ > >>> &((uint8_t *) ip)[1], \ > >>> &((uint8_t *) ip)[2], \ > >>> @@ -1729,4 +1736,8 @@ BUILD_ASSERT_DECL(DNS_HEADER_LEN == sizeof(struct > >>> dns_header)); > >>> #define DNS_CLASS_IN 0x01 > >>> #define DNS_DEFAULT_RR_TTL 3600 > >>> > >>> +#ifdef __cplusplus > >>> +} > >>> +#endif > >>> + > >>> #endif /* packets.h */ > >>> -- > >>> 2.54.0 > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> [email protected] > >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > > > > > -- > > Dmitry > -- Dmitry _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
