diff mbox

[2/2] platform: debug: Simplify ODP_LOG() macro

Message ID 1416827584-21879-3-git-send-email-taras.kondratiuk@linaro.org
State New
Headers show

Commit Message

Taras Kondratiuk Nov. 24, 2014, 11:13 a.m. UTC
Move additional functionality out of ODP_LOG. Move abort() call to
odp_override_log(), so application will be able to implement custom
abort handling.

Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
---
 platform/linux-generic/include/api/odp_debug.h     |   37 ++++----------------
 .../linux-generic/include/odp_debug_internal.h     |    6 ++--
 platform/linux-generic/odp_weak.c                  |    5 ++-
 3 files changed, 14 insertions(+), 34 deletions(-)

Comments

Maxim Uvarov Nov. 24, 2014, 12:05 p.m. UTC | #1
Do weak symbols work also for dynamic linking?

Maxim.

On 11/24/2014 02:13 PM, Taras Kondratiuk wrote:
> Move additional functionality out of ODP_LOG. Move abort() call to
> odp_override_log(), so application will be able to implement custom
> abort handling.
>
> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> ---
>   platform/linux-generic/include/api/odp_debug.h     |   37 ++++----------------
>   .../linux-generic/include/odp_debug_internal.h     |    6 ++--
>   platform/linux-generic/odp_weak.c                  |    5 ++-
>   3 files changed, 14 insertions(+), 34 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h
> index cb5bc83..0fbd6b9 100644
> --- a/platform/linux-generic/include/api/odp_debug.h
> +++ b/platform/linux-generic/include/api/odp_debug.h
> @@ -99,36 +99,8 @@ extern int odp_override_log(odp_log_level_e level, const char *fmt, ...);
>    * ODP LOG macro.
>    */
>   #define ODP_LOG(level, fmt, ...) \
> -do { \
> -	switch (level) { \
> -	case ODP_LOG_ERR: \
> -		odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
> -		__LINE__, __func__, ##__VA_ARGS__); \
> -		break; \
> -	case ODP_LOG_DBG: \
> -		if (ODP_DEBUG_PRINT == 1) \
> -			odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
> -			__LINE__, __func__, ##__VA_ARGS__); \
> -		break; \
> -	case ODP_LOG_PRINT: \
> -		odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
> -		__LINE__, __func__, ##__VA_ARGS__); \
> -		break; \
> -	case ODP_LOG_ABORT: \
> -		odp_override_log(level, "%s:%d:%s(): " fmt, __FILE__, \
> -		__LINE__, __func__, ##__VA_ARGS__); \
> -		abort(); \
> -		break; \
> -	case ODP_LOG_UNIMPLEMENTED: \
> -		odp_override_log(level, \
> -			"%s:%d:The function %s() is not implemented\n" \
> -			fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__); \
> -		break; \
> -	default: \
> -		odp_override_log(level, "Unknown LOG level"); \
> -		break;\
> -	} \
> -} while (0)
> +	odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
> +		__LINE__, __func__, ##__VA_ARGS__)
>   
>   /**
>    * Log print message when the application calls one of the ODP APIs
> @@ -141,7 +113,10 @@ do { \
>    * Log debug message if DEBUG flag is set.
>    */
>   #define ODP_DBG(fmt, ...) \
> -		ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__)
> +	do { \
> +		if (ODP_DEBUG_PRINT == 1) \
> +			ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__);\
> +	} while (0)
>   
>   /**
>    * Log error message.
> diff --git a/platform/linux-generic/include/odp_debug_internal.h b/platform/linux-generic/include/odp_debug_internal.h
> index a87552f..006e8ab 100644
> --- a/platform/linux-generic/include/odp_debug_internal.h
> +++ b/platform/linux-generic/include/odp_debug_internal.h
> @@ -25,8 +25,10 @@ extern "C" {
>   /**
>    * This macro is used to indicate when a given function is not implemented
>    */
> -#define ODP_UNIMPLEMENTED(fmt, ...) \
> -		ODP_LOG(ODP_LOG_UNIMPLEMENTED, fmt, ##__VA_ARGS__)
> +#define ODP_UNIMPLEMENTED() \
> +		odp_override_log(ODP_LOG_UNIMPLEMENTED, \
> +			"%s:%d:The function %s() is not implemented\n" \
> +			__FILE__, __LINE__, __func__)
>   
>   #ifdef __cplusplus
>   }
> diff --git a/platform/linux-generic/odp_weak.c b/platform/linux-generic/odp_weak.c
> index fccbc3f..3cc3d45 100644
> --- a/platform/linux-generic/odp_weak.c
> +++ b/platform/linux-generic/odp_weak.c
> @@ -9,7 +9,7 @@
>   #include <odp_debug_internal.h>
>   #include <odp_hints.h>
>   
> -ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED,
> +ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level,
>   				     const char *fmt, ...)
>   {
>   	va_list args;
> @@ -19,5 +19,8 @@ ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED,
>   	r = vfprintf(stderr, fmt, args);
>   	va_end(args);
>   
> +	if (level == ODP_LOG_ABORT)
> +		abort();
> +
>   	return r;
>   }
Bill Fischofer Nov. 24, 2014, 5:57 p.m. UTC | #2
This article <http://unwiredben.livejournal.com/297826.html> seems relevant
to Maxim's question.

On Mon, Nov 24, 2014 at 6:05 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Do weak symbols work also for dynamic linking?
>
> Maxim.
>
>
> On 11/24/2014 02:13 PM, Taras Kondratiuk wrote:
>
>> Move additional functionality out of ODP_LOG. Move abort() call to
>> odp_override_log(), so application will be able to implement custom
>> abort handling.
>>
>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
>> ---
>>   platform/linux-generic/include/api/odp_debug.h     |   37
>> ++++----------------
>>   .../linux-generic/include/odp_debug_internal.h     |    6 ++--
>>   platform/linux-generic/odp_weak.c                  |    5 ++-
>>   3 files changed, 14 insertions(+), 34 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_debug.h
>> b/platform/linux-generic/include/api/odp_debug.h
>> index cb5bc83..0fbd6b9 100644
>> --- a/platform/linux-generic/include/api/odp_debug.h
>> +++ b/platform/linux-generic/include/api/odp_debug.h
>> @@ -99,36 +99,8 @@ extern int odp_override_log(odp_log_level_e level,
>> const char *fmt, ...);
>>    * ODP LOG macro.
>>    */
>>   #define ODP_LOG(level, fmt, ...) \
>> -do { \
>> -       switch (level) { \
>> -       case ODP_LOG_ERR: \
>> -               odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
>> -               __LINE__, __func__, ##__VA_ARGS__); \
>> -               break; \
>> -       case ODP_LOG_DBG: \
>> -               if (ODP_DEBUG_PRINT == 1) \
>> -                       odp_override_log(level, "%s:%d:%s():" fmt,
>> __FILE__, \
>> -                       __LINE__, __func__, ##__VA_ARGS__); \
>> -               break; \
>> -       case ODP_LOG_PRINT: \
>> -               odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
>> -               __LINE__, __func__, ##__VA_ARGS__); \
>> -               break; \
>> -       case ODP_LOG_ABORT: \
>> -               odp_override_log(level, "%s:%d:%s(): " fmt, __FILE__, \
>> -               __LINE__, __func__, ##__VA_ARGS__); \
>> -               abort(); \
>> -               break; \
>> -       case ODP_LOG_UNIMPLEMENTED: \
>> -               odp_override_log(level, \
>> -                       "%s:%d:The function %s() is not implemented\n" \
>> -                       fmt, __FILE__, __LINE__, __func__,
>> ##__VA_ARGS__); \
>> -               break; \
>> -       default: \
>> -               odp_override_log(level, "Unknown LOG level"); \
>> -               break;\
>> -       } \
>> -} while (0)
>> +       odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
>> +               __LINE__, __func__, ##__VA_ARGS__)
>>     /**
>>    * Log print message when the application calls one of the ODP APIs
>> @@ -141,7 +113,10 @@ do { \
>>    * Log debug message if DEBUG flag is set.
>>    */
>>   #define ODP_DBG(fmt, ...) \
>> -               ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__)
>> +       do { \
>> +               if (ODP_DEBUG_PRINT == 1) \
>> +                       ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__);\
>> +       } while (0)
>>     /**
>>    * Log error message.
>> diff --git a/platform/linux-generic/include/odp_debug_internal.h
>> b/platform/linux-generic/include/odp_debug_internal.h
>> index a87552f..006e8ab 100644
>> --- a/platform/linux-generic/include/odp_debug_internal.h
>> +++ b/platform/linux-generic/include/odp_debug_internal.h
>> @@ -25,8 +25,10 @@ extern "C" {
>>   /**
>>    * This macro is used to indicate when a given function is not
>> implemented
>>    */
>> -#define ODP_UNIMPLEMENTED(fmt, ...) \
>> -               ODP_LOG(ODP_LOG_UNIMPLEMENTED, fmt, ##__VA_ARGS__)
>> +#define ODP_UNIMPLEMENTED() \
>> +               odp_override_log(ODP_LOG_UNIMPLEMENTED, \
>> +                       "%s:%d:The function %s() is not implemented\n" \
>> +                       __FILE__, __LINE__, __func__)
>>     #ifdef __cplusplus
>>   }
>> diff --git a/platform/linux-generic/odp_weak.c
>> b/platform/linux-generic/odp_weak.c
>> index fccbc3f..3cc3d45 100644
>> --- a/platform/linux-generic/odp_weak.c
>> +++ b/platform/linux-generic/odp_weak.c
>> @@ -9,7 +9,7 @@
>>   #include <odp_debug_internal.h>
>>   #include <odp_hints.h>
>>   -ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED,
>> +ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level,
>>                                      const char *fmt, ...)
>>   {
>>         va_list args;
>> @@ -19,5 +19,8 @@ ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e
>> level ODP_UNUSED,
>>         r = vfprintf(stderr, fmt, args);
>>         va_end(args);
>>   +     if (level == ODP_LOG_ABORT)
>> +               abort();
>> +
>>         return r;
>>   }
>>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov Nov. 24, 2014, 6:57 p.m. UTC | #3
On 11/24/2014 08:57 PM, Bill Fischofer wrote:
> This article <http://unwiredben.livejournal.com/297826.html> seems 
> relevant to Maxim's question.

That is interesting. As I understood right author of this article did 
not define default weak function. And in dynamic
linking function is not zero, it's some pointer to ELF jump table. In 
our case we do provide default week function. And
jump to it has to be successful.

Maxim.


>
> On Mon, Nov 24, 2014 at 6:05 AM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Do weak symbols work also for dynamic linking?
>
>     Maxim.
>
>
>     On 11/24/2014 02:13 PM, Taras Kondratiuk wrote:
>
>         Move additional functionality out of ODP_LOG. Move abort() call to
>         odp_override_log(), so application will be able to implement
>         custom
>         abort handling.
>
>         Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org
>         <mailto:taras.kondratiuk@linaro.org>>
>         ---
>           platform/linux-generic/include/api/odp_debug.h  |   37
>         ++++----------------
>           .../linux-generic/include/odp_debug_internal.h  |    6 ++--
>           platform/linux-generic/odp_weak.c |    5 ++-
>           3 files changed, 14 insertions(+), 34 deletions(-)
>
>         diff --git a/platform/linux-generic/include/api/odp_debug.h
>         b/platform/linux-generic/include/api/odp_debug.h
>         index cb5bc83..0fbd6b9 100644
>         --- a/platform/linux-generic/include/api/odp_debug.h
>         +++ b/platform/linux-generic/include/api/odp_debug.h
>         @@ -99,36 +99,8 @@ extern int odp_override_log(odp_log_level_e
>         level, const char *fmt, ...);
>            * ODP LOG macro.
>            */
>           #define ODP_LOG(level, fmt, ...) \
>         -do { \
>         -       switch (level) { \
>         -       case ODP_LOG_ERR: \
>         -               odp_override_log(level, "%s:%d:%s():" fmt,
>         __FILE__, \
>         -               __LINE__, __func__, ##__VA_ARGS__); \
>         -               break; \
>         -       case ODP_LOG_DBG: \
>         -               if (ODP_DEBUG_PRINT == 1) \
>         -                       odp_override_log(level, "%s:%d:%s():"
>         fmt, __FILE__, \
>         -                       __LINE__, __func__, ##__VA_ARGS__); \
>         -               break; \
>         -       case ODP_LOG_PRINT: \
>         -               odp_override_log(level, "%s:%d:%s():" fmt,
>         __FILE__, \
>         -               __LINE__, __func__, ##__VA_ARGS__); \
>         -               break; \
>         -       case ODP_LOG_ABORT: \
>         -               odp_override_log(level, "%s:%d:%s(): " fmt,
>         __FILE__, \
>         -               __LINE__, __func__, ##__VA_ARGS__); \
>         -               abort(); \
>         -               break; \
>         -       case ODP_LOG_UNIMPLEMENTED: \
>         -               odp_override_log(level, \
>         -                       "%s:%d:The function %s() is not
>         implemented\n" \
>         -                       fmt, __FILE__, __LINE__, __func__,
>         ##__VA_ARGS__); \
>         -               break; \
>         -       default: \
>         -               odp_override_log(level, "Unknown LOG level"); \
>         -               break;\
>         -       } \
>         -} while (0)
>         +       odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
>         +               __LINE__, __func__, ##__VA_ARGS__)
>             /**
>            * Log print message when the application calls one of the
>         ODP APIs
>         @@ -141,7 +113,10 @@ do { \
>            * Log debug message if DEBUG flag is set.
>            */
>           #define ODP_DBG(fmt, ...) \
>         -               ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__)
>         +       do { \
>         +               if (ODP_DEBUG_PRINT == 1) \
>         +                       ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__);\
>         +       } while (0)
>             /**
>            * Log error message.
>         diff --git
>         a/platform/linux-generic/include/odp_debug_internal.h
>         b/platform/linux-generic/include/odp_debug_internal.h
>         index a87552f..006e8ab 100644
>         --- a/platform/linux-generic/include/odp_debug_internal.h
>         +++ b/platform/linux-generic/include/odp_debug_internal.h
>         @@ -25,8 +25,10 @@ extern "C" {
>           /**
>            * This macro is used to indicate when a given function is
>         not implemented
>            */
>         -#define ODP_UNIMPLEMENTED(fmt, ...) \
>         -               ODP_LOG(ODP_LOG_UNIMPLEMENTED, fmt, ##__VA_ARGS__)
>         +#define ODP_UNIMPLEMENTED() \
>         +               odp_override_log(ODP_LOG_UNIMPLEMENTED, \
>         +                       "%s:%d:The function %s() is not
>         implemented\n" \
>         +                       __FILE__, __LINE__, __func__)
>             #ifdef __cplusplus
>           }
>         diff --git a/platform/linux-generic/odp_weak.c
>         b/platform/linux-generic/odp_weak.c
>         index fccbc3f..3cc3d45 100644
>         --- a/platform/linux-generic/odp_weak.c
>         +++ b/platform/linux-generic/odp_weak.c
>         @@ -9,7 +9,7 @@
>           #include <odp_debug_internal.h>
>           #include <odp_hints.h>
>           -ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level
>         ODP_UNUSED,
>         +ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level,
>                                              const char *fmt, ...)
>           {
>                 va_list args;
>         @@ -19,5 +19,8 @@ ODP_WEAK_SYMBOL int
>         odp_override_log(odp_log_level_e level ODP_UNUSED,
>                 r = vfprintf(stderr, fmt, args);
>                 va_end(args);
>           +     if (level == ODP_LOG_ABORT)
>         +               abort();
>         +
>                 return r;
>           }
>
>
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Taras Kondratiuk Nov. 25, 2014, 3:09 p.m. UTC | #4
On 11/24/2014 02:05 PM, Maxim Uvarov wrote:
> Do weak symbols work also for dynamic linking?

It does work for dynamic libraries also.

I've found an issue in ODP_UNIMPLEMENTED() macro.
Will post v2.
diff mbox

Patch

diff --git a/platform/linux-generic/include/api/odp_debug.h b/platform/linux-generic/include/api/odp_debug.h
index cb5bc83..0fbd6b9 100644
--- a/platform/linux-generic/include/api/odp_debug.h
+++ b/platform/linux-generic/include/api/odp_debug.h
@@ -99,36 +99,8 @@  extern int odp_override_log(odp_log_level_e level, const char *fmt, ...);
  * ODP LOG macro.
  */
 #define ODP_LOG(level, fmt, ...) \
-do { \
-	switch (level) { \
-	case ODP_LOG_ERR: \
-		odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
-		__LINE__, __func__, ##__VA_ARGS__); \
-		break; \
-	case ODP_LOG_DBG: \
-		if (ODP_DEBUG_PRINT == 1) \
-			odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
-			__LINE__, __func__, ##__VA_ARGS__); \
-		break; \
-	case ODP_LOG_PRINT: \
-		odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
-		__LINE__, __func__, ##__VA_ARGS__); \
-		break; \
-	case ODP_LOG_ABORT: \
-		odp_override_log(level, "%s:%d:%s(): " fmt, __FILE__, \
-		__LINE__, __func__, ##__VA_ARGS__); \
-		abort(); \
-		break; \
-	case ODP_LOG_UNIMPLEMENTED: \
-		odp_override_log(level, \
-			"%s:%d:The function %s() is not implemented\n" \
-			fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__); \
-		break; \
-	default: \
-		odp_override_log(level, "Unknown LOG level"); \
-		break;\
-	} \
-} while (0)
+	odp_override_log(level, "%s:%d:%s():" fmt, __FILE__, \
+		__LINE__, __func__, ##__VA_ARGS__)
 
 /**
  * Log print message when the application calls one of the ODP APIs
@@ -141,7 +113,10 @@  do { \
  * Log debug message if DEBUG flag is set.
  */
 #define ODP_DBG(fmt, ...) \
-		ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__)
+	do { \
+		if (ODP_DEBUG_PRINT == 1) \
+			ODP_LOG(ODP_LOG_DBG, fmt, ##__VA_ARGS__);\
+	} while (0)
 
 /**
  * Log error message.
diff --git a/platform/linux-generic/include/odp_debug_internal.h b/platform/linux-generic/include/odp_debug_internal.h
index a87552f..006e8ab 100644
--- a/platform/linux-generic/include/odp_debug_internal.h
+++ b/platform/linux-generic/include/odp_debug_internal.h
@@ -25,8 +25,10 @@  extern "C" {
 /**
  * This macro is used to indicate when a given function is not implemented
  */
-#define ODP_UNIMPLEMENTED(fmt, ...) \
-		ODP_LOG(ODP_LOG_UNIMPLEMENTED, fmt, ##__VA_ARGS__)
+#define ODP_UNIMPLEMENTED() \
+		odp_override_log(ODP_LOG_UNIMPLEMENTED, \
+			"%s:%d:The function %s() is not implemented\n" \
+			__FILE__, __LINE__, __func__)
 
 #ifdef __cplusplus
 }
diff --git a/platform/linux-generic/odp_weak.c b/platform/linux-generic/odp_weak.c
index fccbc3f..3cc3d45 100644
--- a/platform/linux-generic/odp_weak.c
+++ b/platform/linux-generic/odp_weak.c
@@ -9,7 +9,7 @@ 
 #include <odp_debug_internal.h>
 #include <odp_hints.h>
 
-ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED,
+ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level,
 				     const char *fmt, ...)
 {
 	va_list args;
@@ -19,5 +19,8 @@  ODP_WEAK_SYMBOL int odp_override_log(odp_log_level_e level ODP_UNUSED,
 	r = vfprintf(stderr, fmt, args);
 	va_end(args);
 
+	if (level == ODP_LOG_ABORT)
+		abort();
+
 	return r;
 }