Hi Elad,
On 2018/07/17 1:35, Elad Lahav wrote:
> On Mon, Jul 16, 2018 at 11:42 AM, Akira Yokosawa <akiyks@xxxxxxxxx> wrote:
>> First, two helpers are defined as follows:
>>
>> #define handle_error_en(en, msg) \
>> do { errno = en; perror(msg); exit(EXIT_FAILURE); } while (0)
>>
>> #define handle_error(msg) \
>> do { perror(msg); exit(EXIT_FAILURE); } while (0)
>>
>> Then, one of the call sites looks as follows:
>>
>> s = pthread_create(&tinfo[tnum].thread_id, &attr,
>> &thread_start, &tinfo[tnum]);
>> if (s != 0)
>> handle_error_en(s, "pthread_create");
>>
>> If we employ this pattern, one of the hunks in your patch will look like:
>>
>> - if (pthread_mutex_lock(pmlp) != 0) {
>> - perror("lock_reader:pthread_mutex_lock");
>> - exit(-1);
>> - }
>> + if ((en = pthread_mutex_lock(pmlp)) != 0)
>> + handle_error_en(en, "lock_reader:pthread_mutex_lock");
>>
>> Thoughts?
>
> You are, of course, free to do as you want with your book, but I would
> advise against the proposal. Novice software developers will often
> copy patterns from books, which means that the examples need to be
> held to a higher standard. I do agree that error handling is not the
> point of these examples, so you shouldn't spend too much time on it,
> but at least at one point it should show the correct pattern. The rest
> of the code can just have a "// Report error and exit" comment.
> If you do want a helper, then a better solution would be:
>
> static inline void __attribute__((noreturn))
> fatal(char const * const msg, int const err)
> {
> fprintf(stderr, "%s: %s\n", msg, strerror(err));
> exit(EXIT_FAILURE);
> }
>
> The name 'fatal' better conveys to the reader of the code the fact
> that the call doesn't return. The snippet also demonstrates a more
> modern approach to C code (use of inline functions, const, function
> attributes).
Thank you for your feedback!
I agree with you that inline functions are better choice.
You might also want to update the example in the man pages? ;-)
Would function names of "fatal_en()" and "fatal()" as defined below
work with you?
static inline void __attribute__((noreturn))
fatal_en(char const * const msg, int const err)
{
fprintf(stderr, "%s: %s\n", msg, strerror(err));
exit(EXIT_FAILURE);
}
static inline void __attribute__((noreturn))
fatal(char const * const msg)
{
perror(msg);
exit(EXIT_FAILURE);
}
Thanks, Akira
>
> --Elad
>
--
To unsubscribe from this list: send the line "unsubscribe perfbook" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
