lkml.org 
[lkml]   [2008]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: New distributed storage release.
On Mon, Sep 08, 2008 at 08:19:23PM +0200, Sven Wegener (sven.wegener@stealer.net) wrote:
> > > drivers/block/dst/trans.c:160: warning: format '%llu' expects type 'long long unsigned int', but argument 4 has type 'sector_t'
> >
> > Yup, sector_t is diffrent depending on arch and config options (u64 vs unsigned long),
> > so it is not possible to represent it correctly without dereferencing
> > to another type.
>
> Yup, cast it. That are a lot of warnings and best to avoid them.

Yes, warnings have to be removed.

> > > uhm, menuconfig, and then just the debug option there seems wrong.
> >
> > In case of extended functionality there will be no need to change config
> > options, now it looks like single string in the higher layer config.
>
> If there is coming more funtionality, ok, but currently it's irritating to
> have a submenu and then just a single debug option in there.

Nothing is actually planned, I do not care which config option will be
used for that, just do not want to get a new claims on that it should be
changed.

> Yeah, the dprintk()s are ok, but I was after the printk()s that follow.
> They are unconditional and will always print the key.

Ugh, you are right, I missed that hunk.

> > > sizeof(*req) is preferred, likewise for other sizeof() uses in the code.
> >
> > No, I do belive that grepping over |struct ablkcipher_request| when
> > something is about to be changed is more convenient, than searching for
> > the structure name and then object name itself.
> > It is matter of a taste though.
>
> Yeah, but in nearly all cases you don't need to change anything in that
> line, when you use sizeof(*ptr), because it always gets you the correct
> size you want. There are cases we could argue about, because you might
> want to do a limited memset() or memcpy() with a different size, but for
> most cases like memset() to zero or kmalloc, sizeof(*ptr) makes it clear
> that you want to clear out the whole structure or you want enough memory
> to store the type of ptr in it. Don't know how strict the policy is, but
> it's part of CodingStyle.

Fortunately I missed that flamewar about coding style.

> > I just want to shut up the compiler, since failing to register
> > informational attribute is not critical. But it could also be
> > used to fall the initialization.
>
> For just shutting up the compiler, use the void cast. I think removing a
> failed attribute is ok.

If I will ut (void) casting there, I'm pretty sure I will be blamed
not to check th return values, now I check it :)

> > 36 bytes iirc - not that small for stack allocation I think.
>
> In other cases you allocate u8 iv[32] on the stack, so I guess this point
> is void. And I think 36 bytes isn't that much and it looks like you're
> called from user space, so I don't think you have a deep callchain here.

That's a fast path, so I optimized a bit, but I agree, iv initializaion
could be also moved to the preallocated area.
I think connector message can be allocated on stack, since I know the
whole callchain in that path.

I will updated the patch.

--
Evgeniy Polyakov


\
 
 \ /
  Last update: 2008-09-08 21:35    [W:0.569 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site