[sheepdog] [PATCH RFC] sheep: remove assert() for better error messages

[Hitoshi Mitake]

On Wed, Oct 3, 2012 at 11:26 AM, MORITA Kazutaka
<morita.kazutaka at lab.ntt.co.jp> wrote:
> At Mon,  1 Oct 2012 22:34:25 +0900,
> Hitoshi Mitake wrote:
>>
>> diff --git a/include/sd_assert.h b/include/sd_assert.h
>> new file mode 100644
>> index 0000000..05c553e
>> --- /dev/null
>> +++ b/include/sd_assert.h
>> @@ -0,0 +1,31 @@
>> +#ifndef SD_ASSERT_H
>> +#define SD_ASSERT_H
>> +
>> +#include "logger.h"
>> +#include "daemonize.h"
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +
>> +#ifdef assert
>> +#error "don't include assert.h directly"
>> +#endif
>> +
>> +#define assert(expr) do {                                            \
>> +             if (expr)                                               \
>> +                     break;                                          \
>> +                                                                     \
>> +             if (is_daemonized()) {                                  \
>> +                     vprintf(SDOG_ERR, "assert: %s:%d: %s: "         \
>> +                             "Asserting `%s' failed.\n",             \
>> +                             __FILE__, __LINE__, __func__, #expr);   \
>> +             } else {                                                \
>> +                     fprintf(stderr, "assert: %s:%d: %s: "           \
>> +                             "Asserting `%s' failed.\n",             \
>> +                             __FILE__, __LINE__, __func__, #expr);   \
>> +                      }                                              \
>> +                                                                     \
>> +             abort();                                                \
>> +     } while (0)
>> +
>> +#endif       /* SD_ASSERT_H */
>
> Some comments:
>
>  - assert() should do nothing when NDEBUG is defined.
>  - I think the log level should be SDOG_EMERG rather than SDOG_ERR.
>  - How about using vprintf in any case?  vprintf prints message to
>    stderr when the program is running in foreground.
>  - How about define assert in lib/util.h directly?
>
> Then, I think the definition should be something like the following.
>
> =
> #ifdef assert
> #undef assert
> #endif
>
> #ifdef NDEBUG
> #define assert(e)  ((void)0)
> #else
> #define assert(e)  (!(e) && panic(....))
> #endif
> ==

Thanks for your comments.
 I'll update my patch based on your comments and post it later.


Thanks,
Hitoshi