Hi All, This thread has dissolved into unnecessary time-wasting on nitpick changes. There is no technical issue with uint32_t, so this patch remains as is, and this should be accepted for merge.
If you feel differently, reply to this with a detailed description of a genuine technical bug. Regards, -Harry From: Amber, Kumar <[email protected]> Sent: Wednesday, July 7, 2021 11:04 AM To: Eelco Chaudron <[email protected]>; Van Haaren, Harry <[email protected]> Cc: Ferriter, Cian <[email protected]>; [email protected]; [email protected]; [email protected]; Stokes, Ian <[email protected]> Subject: RE: [v6 00/11] MFEX Infrastructure + Optimizations Hi Eelco, I tried with the suggestion “zd” is deprecated and in place of it %"PRIdSIZE`` is mentioned which still causes build failure on non-ssl 32 bit builds. Regards Amber From: Eelco Chaudron <[email protected]<mailto:[email protected]>> Sent: Wednesday, July 7, 2021 3:02 PM To: Van Haaren, Harry <[email protected]<mailto:[email protected]>> Cc: Amber, Kumar <[email protected]<mailto:[email protected]>>; Ferriter, Cian <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>; Stokes, Ian <[email protected]<mailto:[email protected]>> Subject: Re: [v6 00/11] MFEX Infrastructure + Optimizations On 7 Jul 2021, at 11:09, Van Haaren, Harry wrote: -----Original Message----- From: Eelco Chaudron <[email protected]<mailto:[email protected]>> Sent: Wednesday, July 7, 2021 9:35 AM To: Amber, Kumar <[email protected]<mailto:[email protected]>> Cc: Ferriter, Cian <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>; [email protected]<mailto:[email protected]>; Van Haaren, Harry <[email protected]<mailto:[email protected]>>; Stokes, Ian <[email protected]<mailto:[email protected]>> Subject: Re: [v6 00/11] MFEX Infrastructure + Optimizations On 6 Jul 2021, at 17:06, Amber, Kumar wrote: Hi Eelco , Here is the diff vor v6 vs v5 : Patch 1 : diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c index 1aebf3656d..4987d628a4 100644 --- a/lib/dpif-netdev-private-extract.c +++ b/lib/dpif-netdev-private-extract.c @@ -233,7 +233,7 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, uint32_t keys_size, odp_port_t in_port, struct dp_netdev_pmd_thread *pmd_handle) { - const size_t cnt = dp_packet_batch_size(packets); + const uint32_t cnt = dp_packet_batch_size(packets); uint16_t good_l2_5_ofs[NETDEV_MAX_BURST]; uint16_t good_l3_ofs[NETDEV_MAX_BURST]; uint16_t good_l4_ofs[NETDEV_MAX_BURST]; @@ -247,7 +247,7 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, atomic_uintptr_t *pmd_func = (void *)&pmd->miniflow_extract_opt; atomic_store_relaxed(pmd_func, (uintptr_t) default_func); VLOG_ERR("Invalid key size supplied, Key_size: %d less than" - "batch_size: %ld", keys_size, cnt); + "batch_size: %d", keys_size, cnt); What was the reason for changing this size_t to uint32_t? Is see other instances where %ld is used for logging? And other functions like dp_netdev_run_meter() have it as a size_t? The reason to change this is because 32-bit builds were breaking due to incorrect format-specifier in the printf. Root cause is because size_t requires different printf format specifier based on 32 or 64 bit arch. (As you likely know, size_t is to describe objects in memory, or the return of sizeof operator. Because 32-bit and 64-bit can have different amounts of memory, size_t can be "unsigned int" or "unsigned long long"). It does not make sense to me to use a type of variable that changes width based on architecture to count batch size (a value from 0 to 32). Simplicity and obvious-ness is nice, and a uint32_t is always exactly what you read it to be, and %d will always be correct for uint32_t regardless of 32 or 64 bit. We should not change this back to the more complex and error-prone "size_t", uint32_t is better. I don't think it’s more error-prone if the right type qualifier is used, i.e. %zd. See also the coding style document, so I would suggest changing it to: @@ -233,7 +233,7 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, uint32_t keys_size, odp_port_t in_port, struct dp_netdev_pmd_thread *pmd_handle) { * const uint32_t cnt = dp_packet_batch_size(packets); * const size_t cnt = dp_packet_batch_size(packets); uint16_t good_l2_5_ofs[NETDEV_MAX_BURST]; uint16_t good_l3_ofs[NETDEV_MAX_BURST]; uint16_t good_l4_ofs[NETDEV_MAX_BURST]; @@ -247,7 +247,7 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, atomic_uintptr_t *pmd_func = (void *)&pmd->miniflow_extract_opt; atomic_store_relaxed(pmd_func, (uintptr_t) default_func); VLOG_ERR("Invalid key size supplied, Key_size: %d less than" · "batch_size: %d", keys_size, cnt); * · "batch_size: %"PRIdSIZE, keys_size, cnt); · return 0; * } <snip> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
