[Freeipa-devel] [PATCH 0093] Log memory allocation failures

[Petr Spacek]

On 11/16/2012 02:24 PM, Simo Sorce wrote:
> On Fri, 2012-11-16 at 13:31 +0100, Petr Spacek wrote:
>> +#define FILE_LINE_FUNC_MSG "%-15s: %4d: %-20s"
>> +#define FILE_LINE_FUNC_ARG __FILE__, __LINE__, __func__
>
> Wouldn't it be more compact and less error prone to do something like:
>
> #define LOG_POSITION_MSG(str, ...) \
> do { \
> log_error("[%-15s: %4d: %-20s] " str, \
> 	  __FILE__, __LINE__, __func__, __VA_ARGS__); \
> } while(0);
>
>>   #define CLEANUP_WITH(result_code)                              \
>>          do {                                                    \
>>                  result = (result_code);                         \
>> @@ -46,15 +52,21 @@
>>                  (target_ptr) = isc_mem_allocate((m), (s));      \
>>                  if ((target_ptr) == NULL) {                     \
>>                          result = ISC_R_NOMEMORY;                \
>> +                       log_error("MEMORY ALLOCATION FAILURE at
>> "       \
>> +                                 FILE_LINE_FUNC_MSG,           \
>> +                                 FILE_LINE_FUNC_ARG);  \
>
> and here simply:
> 	LOG_POSITION_MSG("MEMORY ALLOCATION FAILURE");
>
> The _MSG seem misleading doesn't make you think it is a format string
> (maybe calling it _FMT would be better if you want to keep this split in
> pieces).
>
> The single form above let you write less and still do things like:
> LOG_POSITION_MSG("This failed with %d (%s)", ret, strerror(ret));

Yes, it would. I split it to avoid dns_result_totext() duplication in my next 
patch ... I see this approach as better now. Amended patch is attached.

-- 
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0093-2-Log-memory-allocation-failures.patch
Type: text/x-patch
Size: 2426 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121116/4f646362/attachment.bin>