From: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
To: Sven Wegener <sven.wegener@stealer.net>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: New distributed storage release.
Date: Mon, 8 Sep 2008 23:31:32 +0400 [thread overview]
Message-ID: <20080908193132.GB15405@2ka.mipt.ru> (raw)
In-Reply-To: <alpine.LNX.2.00.0809081948450.19109@titan.stealer.net>
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
prev parent reply other threads:[~2008-09-08 19:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-08 15:22 New distributed storage release Evgeniy Polyakov
2008-09-08 17:19 ` Sven Wegener
2008-09-08 17:43 ` Evgeniy Polyakov
2008-09-08 18:19 ` Sven Wegener
2008-09-08 19:31 ` Evgeniy Polyakov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080908193132.GB15405@2ka.mipt.ru \
--to=johnpol@2ka.mipt.ru \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sven.wegener@stealer.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).