Re: [Xen-devel] [PATCH v2] xmalloc: add support for checking the pool integrity

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.12.14 at 17:00, <mdontu@xxxxxxxxxxxxxxx> wrote:
> On Monday 08 December 2014 10:18:01 Jan Beulich wrote:
>> >>> On 08.12.14 at 03:30, <mdontu@xxxxxxxxxxxxxxx> wrote:
>> > +#ifndef NDEBUG
>> > +static bool_t xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
>> > +{
>> > +    while ( b )
>> > +    {
>> > +        int __fl;
>> > +        int __sl;
>> > +
>> > +        MAPPING_INSERT(b->size, &__fl, &__sl);
>> > +        if ( __fl != fl || __sl != sl )
>> > +        {
>> > +            printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = 
> %d, sl = %d } should be { fl = %d, sl = %d }\n",
>> 
>> Quoting my reply to v1: "Long line. Only the format message alone
>> is allowed to exceed 80 characters."
>> 
> 
> Just so I don't send another faulty patch, you would see that printk()
> being:
> 
>   printk(XENLOG_ERR
>          "xmem_pool: for block %p size = %u, { fl = %d, sl = %d } should be { 
> fl = %d, sl = %d }\n",
>          b, b->size, fl, sl, __fl, __sl);
> 
> ?

Yes. And I'd also recommend removing the spaces around the =
characters in the format string. Considering the message being a
debugging one, perhaps the two pairs could be printed without
any extras, i.e. just {x,y}.

>> Also with there potentially being multiple pools, shouldn't all of the
>> log messages the patch issues be extended to allow identifying the
>> offending one?
>> 
> 
> I think I can insert the pool name in that message too. Something like:
> 
>   printk(XENLOG_ERR
>          "xmem_pool: %s: for block [...]\n",
>          pool->name, b, b->size [...]);
> 
> would do? A quick preview:

Yes. I very think that's what the name's for after all.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel