Remove unnecessary kmalloc casts in drivers/char/tty_io.c
Signed-off-by: Ahmed Darwish <[email protected]>
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 47a6eac..97f54b0 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1932,16 +1932,14 @@ static int init_dev(struct tty_driver *driver, int idx,
}
if (!*tp_loc) {
- tp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
- GFP_KERNEL);
+ tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
if (!tp)
goto free_mem_out;
*tp = driver->init_termios;
}
if (!*ltp_loc) {
- ltp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
- GFP_KERNEL);
+ ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
if (!ltp)
goto free_mem_out;
memset(ltp, 0, sizeof(struct ktermios));
@@ -1965,16 +1963,14 @@ static int init_dev(struct tty_driver *driver, int idx,
}
if (!*o_tp_loc) {
- o_tp = (struct ktermios *)
- kmalloc(sizeof(struct ktermios), GFP_KERNEL);
+ o_tp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
if (!o_tp)
goto free_mem_out;
*o_tp = driver->other->init_termios;
}
if (!*o_ltp_loc) {
- o_ltp = (struct ktermios *)
- kmalloc(sizeof(struct ktermios), GFP_KERNEL);
+ o_ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
if (!o_ltp)
goto free_mem_out;
memset(o_ltp, 0, sizeof(struct ktermios));
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
On Fri, 5 Jan 2007, Ahmed S. Darwish wrote:
> Remove unnecessary kmalloc casts in drivers/char/tty_io.c
rather than remove these casts a file or two at a time, why not just
do them all at once and submit a single patch? there aren't that many
of them:
grep -Er "\([^\)\*]+\*\) ?k[cmz]alloc ?\(" .
rday
On Fri, Jan 05, 2007 at 01:56:09AM -0500, Robert P. J. Day wrote:
> On Fri, 5 Jan 2007, Ahmed S. Darwish wrote:
>
> > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
>
> rather than remove these casts a file or two at a time, why not just
> do them all at once and submit a single patch? there aren't that many
> of them:
OK, Thanks for the tip ..
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
Ahmed S. Darwish wrote:
> Remove unnecessary kmalloc casts in drivers/char/tty_io.c
>
> Signed-off-by: Ahmed Darwish <[email protected]>
> if (!*ltp_loc) {
> - ltp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
> - GFP_KERNEL);
> + ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
^^^^^^^
> if (!ltp)
> goto free_mem_out;
> memset(ltp, 0, sizeof(struct ktermios));
^^^^^^
kzalloc
> if (!*o_ltp_loc) {
> - o_ltp = (struct ktermios *)
> - kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> + o_ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
^^^^^^^
> if (!o_ltp)
> goto free_mem_out;
> memset(o_ltp, 0, sizeof(struct ktermios));
^^^^^^
kzalloc
HTH
Eike
On Fri, Jan 05, 2007 at 09:10:01AM +0100, Rolf Eike Beer wrote:
> Ahmed S. Darwish wrote:
> > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
> >
> > Signed-off-by: Ahmed Darwish <[email protected]>
>
> if (!*ltp_loc) {
> - ltp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
> - GFP_KERNEL);
> + ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> ^^^^^^^
> if (!ltp)
> goto free_mem_out;
> memset(ltp, 0, sizeof(struct ktermios));
> ^^^^^^
> kzalloc
>
> if (!*o_ltp_loc) {
> - o_ltp = (struct ktermios *)
> - kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> + o_ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> ^^^^^^^
> if (!o_ltp)
> goto free_mem_out;
> memset(o_ltp, 0, sizeof(struct ktermios));
> ^^^^^^
> kzalloc
Currently I'm dropping this patch and writing a big patch to remove all the
k[mzc]alloc castings in the 20-rc3 tree as suggested by Mr. Robert Day.
I think this will be better done in another patch to let every patch do one
single thing. right ?
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
On Fri, 5 Jan 2007, Ahmed S. Darwish wrote:
> On Fri, Jan 05, 2007 at 09:10:01AM +0100, Rolf Eike Beer wrote:
> > Ahmed S. Darwish wrote:
> > > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
> > >
> > > Signed-off-by: Ahmed Darwish <[email protected]>
> >
> > if (!*ltp_loc) {
> > - ltp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
> > - GFP_KERNEL);
> > + ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > ^^^^^^^
> > if (!ltp)
> > goto free_mem_out;
> > memset(ltp, 0, sizeof(struct ktermios));
> > ^^^^^^
> > kzalloc
> >
> > if (!*o_ltp_loc) {
> > - o_ltp = (struct ktermios *)
> > - kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > + o_ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > ^^^^^^^
> > if (!o_ltp)
> > goto free_mem_out;
> > memset(o_ltp, 0, sizeof(struct ktermios));
> > ^^^^^^
> > kzalloc
>
> Currently I'm dropping this patch and writing a big patch to remove
> all the k[mzc]alloc castings in the 20-rc3 tree as suggested by Mr.
> Robert Day. I think this will be better done in another patch to let
> every patch do one single thing. right ?
almost. as i've learned (the hard way), while each patch should
logically accomplish one thing, while you're there, you might as well
clean up other issues *at the same locations*.
in this case, as mr. beer suggests, you should also check if this
represents a kmalloc->kzalloc cleanup (there's lots of those), and
also see if you can replace one of these:
sizeof(struct blah)
with one of these:
sizeof(*blahptr)
according to the CodingStyle guide.
rday
p.s. just FYI, i have a patch that does most of this, but i was going
to hold off submitting it until 2.6.20 had arrived. but if you want
to take a shot at it, it's all yours.
Am Freitag, 5. Januar 2007 11:06 schrieb Ahmed S. Darwish:
> On Fri, Jan 05, 2007 at 09:10:01AM +0100, Rolf Eike Beer wrote:
> > Ahmed S. Darwish wrote:
> > > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
> > >
> > > Signed-off-by: Ahmed Darwish <[email protected]>
> >
> > if (!*ltp_loc) {
> > - ltp = (struct ktermios *) kmalloc(sizeof(struct ktermios),
> > - GFP_KERNEL);
> > + ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > ^^^^^^^
> > if (!ltp)
> > goto free_mem_out;
> > memset(ltp, 0, sizeof(struct ktermios));
> > ^^^^^^
> > kzalloc
> >
> > if (!*o_ltp_loc) {
> > - o_ltp = (struct ktermios *)
> > - kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > + o_ltp = kmalloc(sizeof(struct ktermios), GFP_KERNEL);
> > ^^^^^^^
> > if (!o_ltp)
> > goto free_mem_out;
> > memset(o_ltp, 0, sizeof(struct ktermios));
> > ^^^^^^
> > kzalloc
>
> Currently I'm dropping this patch and writing a big patch to remove all the
> k[mzc]alloc castings in the 20-rc3 tree as suggested by Mr. Robert Day.
One big patch for the whole kernel will not work anyway. You have to split it
up to allow subsystems to integrate them in their own trees. With one big
patch you would get collisions all over the tree causing the complete patch
to get dropped. Also CC subsystem maintainers on their parts. And please send
the patches as replies to the first one as it cleans up readability of lkml a
lot :)
> I think this will be better done in another patch to let every patch do one
> single thing. right ?
Yes. But I would suggest starting with the kmalloc()->kzalloc() things. When
you do this conversions just remove the casts of the lines you're touching.
This will reduce the size of the complete thing avoiding two rather trivial
patches touching the same line twice.
Eike
On Fri, Jan 05, 2007 at 11:26:07AM +0100, Rolf Eike Beer wrote:
> One big patch for the whole kernel will not work anyway. You have to split it
> up to allow subsystems to integrate them in their own trees. With one big
> patch you would get collisions all over the tree causing the complete patch
> to get dropped. Also CC subsystem maintainers on their parts. And please send
> the patches as replies to the first one as it cleans up readability of lkml a
> lot :)
Oops, Just read this warning after sending the (big) patch. Sorry It's my first
patch :). I'll split it and do as written. Thanks alot :).
> > I think this will be better done in another patch to let every patch do one
> > single thing. right ?
>
> Yes. But I would suggest starting with the kmalloc()->kzalloc() things. When
> you do this conversions just remove the casts of the lines you're touching.
> This will reduce the size of the complete thing avoiding two rather trivial
> patches touching the same line twice.
>
> Eike
OK. In progress
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
On 1/5/07, Robert P. J. Day <[email protected]> wrote:
> in this case, as mr. beer suggests, you should also check if this
> represents a kmalloc->kzalloc cleanup (there's lots of those), and
> also see if you can replace one of these:
>
> sizeof(struct blah)
>
> with one of these:
>
> sizeof(*blahptr)
>
> according to the CodingStyle guide.
Unfortunately, not all maintainers like sizeof() changes, so you're
better off leaving them as-is.
> represents a kmalloc->kzalloc cleanup (there's lots of those), and
> also see if you can replace one of these:
>
> sizeof(struct blah)
>
> with one of these:
>
> sizeof(*blahptr)
Patches that do this will get rejected by the tty maintainer in favour of
the clarity of the sizeof(struct xyz) format 8)
Ahmed - if you can send me a patch for the tty_io/tty_ioctl code which
switches to kzalloc where it makes sense and removes un-needed casts I'll
review it and push the bits that look sane upstream.
Alan
On Fri, 5 Jan 2007, Alan wrote:
> > represents a kmalloc->kzalloc cleanup (there's lots of those), and
> > also see if you can replace one of these:
> >
> > sizeof(struct blah)
> >
> > with one of these:
> >
> > sizeof(*blahptr)
>
> Patches that do this will get rejected by the tty maintainer in favour of
> the clarity of the sizeof(struct xyz) format 8)
ok, i was just going by the recommendations of the CodingStyle guide.
rday
On Fri, Jan 05, 2007 at 10:51:13AM +0000, Alan wrote:
> Ahmed - if you can send me a patch for the tty_io/tty_ioctl code which
> switches to kzalloc where it makes sense and removes un-needed casts I'll
> review it and push the bits that look sane upstream.
>
> Alan
OK I'll try this now.
Are you Mr. Alan Cox. ?. Sorry, "Alan" alone is ambiguous.
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
Am Freitag, 5. Januar 2007 11:32 schrieb Ahmed S. Darwish:
> On Fri, Jan 05, 2007 at 11:26:07AM +0100, Rolf Eike Beer wrote:
> > One big patch for the whole kernel will not work anyway. You have to
> > split it up to allow subsystems to integrate them in their own trees.
> > With one big patch you would get collisions all over the tree causing the
> > complete patch to get dropped. Also CC subsystem maintainers on their
> > parts. And please send the patches as replies to the first one as it
> > cleans up readability of lkml a lot :)
>
> Oops, Just read this warning after sending the (big) patch. Sorry It's my
> first patch :). I'll split it and do as written. Thanks alot :).
That wasn't meant for resending. If you resend the whole series it's good to
start a new thread. But if you have several related patches (usually this
[PATCH N/xx] thing) it helps if you either send a 0/xx first describing the
whole series or at least sending everything as reply to 1/xx. This way the
mail program will help everybody by keeping this things together.
Eike
> On Fri, Jan 05, 2007 at 09:10:01AM +0100, rday wrote:
> > Ahmed S. Darwish wrote:
> > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
> > Signed-off-by: Ahmed Darwish <[email protected]>
>
> rday
>
> p.s. just FYI, i have a patch that does most of this, but i was going
> to hold off submitting it until 2.6.20 had arrived. but if you want
> to take a shot at it, it's all yours.
OK, then I should stop sending new patches related to that matter to avoid
patch conflicts. right ?
--
Ahmed S. Darwish
http://darwish-07.blogspot.com
On Sat, 6 Jan 2007, Ahmed S. Darwish wrote:
> > On Fri, Jan 05, 2007 at 09:10:01AM +0100, rday wrote:
> > > Ahmed S. Darwish wrote:
> > > Remove unnecessary kmalloc casts in drivers/char/tty_io.c
> > > Signed-off-by: Ahmed Darwish <[email protected]>
> >
> > rday
> >
> > p.s. just FYI, i have a patch that does most of this, but i was going
> > to hold off submitting it until 2.6.20 had arrived. but if you want
> > to take a shot at it, it's all yours.
>
> OK, then I should stop sending new patches related to that matter to
> avoid patch conflicts. right ?
no, no, go ahead. i have enough to do to keep me busy.
rday