Re: svn commit: r1916337 - /apr/apr/trunk/include/apr_general.h

On 3/15/24 2:35 PM, [email protected] wrote:
> Author: ylavic
> Date: Fri Mar 15 13:35:04 2024
> New Revision: 1916337
> 
> URL: http://svn.apache.org/viewvc?rev=1916337&view=rev
> Log:
> apr_general: Use __builtin_offsetof for APR_OFFSETOF if available.  PR 68763.
> 
> Sanitizers might warn about &((type *)0)->field being UB, so use the builtin
> which is meant to avoid it.
> 
> 
> Modified:
>     apr/apr/trunk/include/apr_general.h
> 
> Modified: apr/apr/trunk/include/apr_general.h
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_general.h?rev=1916337&r1=1916336&r2=1916337&view=diff
> ==============================================================================
> --- apr/apr/trunk/include/apr_general.h (original)
> +++ apr/apr/trunk/include/apr_general.h Fri Mar 15 13:35:04 2024
> @@ -106,7 +106,9 @@ typedef enum { APR_WAIT_READ, APR_WAIT_W
>   * @param field  data field within the structure
>   * @return offset
>   */
> -#if defined(offsetof) && !defined(__cplusplus)
> +#if defined(__has_builtin) && __has_builtin(__builtin_offsetof)

This causes an

./include/apr_general.h:109:45: error: missing binary operator before token "("

with gcc 8.5.0 for me. Following https://gcc.gnu.org/onlinedocs/cpp/_005f_005fhas_005fbuiltin.html
and changing it to

Index: include/apr_general.h
===================================================================
--- include/apr_general.h	(revision 1916382)
+++ include/apr_general.h	(working copy)
@@ -106,13 +106,18 @@
  * @param field  data field within the structure
  * @return offset
  */
-#if defined(__has_builtin) && __has_builtin(__builtin_offsetof)
+#if defined(__has_builtin)
+#if __has_builtin(__builtin_offsetof)
 #define APR_OFFSETOF(s_type,field) __builtin_offsetof(s_type,field)
-#elif defined(offsetof) && !defined(__cplusplus)
+#endif
+#endif
+#if !defined(APR_OFFSETOF)
+#if defined(offsetof) && !defined(__cplusplus)
 #define APR_OFFSETOF(s_type,field) offsetof(s_type,field)
 #else
 #define APR_OFFSETOF(s_type,field) APR_OFFSET(s_type*,field)
 #endif
+#endif


Looks like to me that __has_builtin does not like to be used within an '&&' operation for whatever reason.

> +#define APR_OFFSETOF(s_type,field) __builtin_offsetof(s_type,field)
> +#elif defined(offsetof) && !defined(__cplusplus)
>  #define APR_OFFSETOF(s_type,field) offsetof(s_type,field)
>  #else
>  #define APR_OFFSETOF(s_type,field) APR_OFFSET(s_type*,field)
> 
> 
> 

Regards

Rüdiger