Re: [PATCH 07/14] DRBD: internal_data_structures

[Philipp Reisner <philipp.reisner@linbit.com>]

On Sunday 12 April 2009 18:44:53 Bart Van Assche wrote:
> On Fri, Apr 10, 2009 at 2:12 PM, Philipp Reisner
>
> <philipp.reisner@linbit.com> wrote:
> > diff -uNrp linux-2.6.30-rc1/drivers/block/drbd/drbd_int.h
> > linux-2.6.30-rc1-drbd/drivers/block/drbd/drbd_int.h
>
> ...
>
> > +#define PRINTK(level, fmt, args...) \
> > +       printk(level "drbd%d: " fmt, \
> > +               mdev->minor , ##args)
>
> The above looks similar to dev_printk() from <linux/device.h>. It
> would be great if some of the DRBD-specific macro's could be replaced
> by existing Linux macro's. This would improve the readability of the
> DRBD source code.
>

Ok. I have done that. Although the dev_err() family of function produce
log that precedes each line with the word "block". Rather useless, but
okay. No more DRBD specific INFO() ERR() macros.

http://git.drbd.org/?p=linux-2.6-drbd.git;a=commit;h=54d19bcccce597742c8883c2b372f0d799dd3c67

> ...
>
> > +#define D_ASSERT(exp)  if (!(exp)) \
> > +        ERR("ASSERT( " #exp " ) in %s:%d\n", __FILE__, __LINE__)
> > +
> > +#define ERR_IF(exp) if (({                             \
> > +       int _b = (exp) != 0;                            \
> > +       if (_b) ERR("%s: (%s) in %s:%d\n",              \
> > +               __func__, #exp, __FILE__, __LINE__);    \
> > +        _b;                                            \
> > +       }))
>
> How about replacing invocations of D_ASSERT() and/or ERR_IF() by WARN_ON()
> ?

I would rather prefer to keep the ASSERT() semantic, since that is a 
widely used concept. -- The existing kernel code already has quite a
few definition of subsystem's incarnation of ASSERTS variants. ;)

Removing ERR_IF() goes onto the TODO list.

-phil
-- 
: Dipl-Ing Philipp Reisner
: LINBIT | Your Way to High Availability
: Tel: +43-1-8178292-50, Fax: +43-1-8178292-82
: http://www.linbit.com

DRBD(R) and LINBIT(R) are registered trademarks of LINBIT, Austria.