Em Tue, Aug 08, 2000 at 02:39:08PM -0500, Bill Wendling escreveu:
> Also sprach Arnaldo Carvalho de Melo:
> } /* Create the PC card device object. */
> } link = kmalloc(sizeof(struct dev_link_t), GFP_KERNEL);
> } - memset(link, 0, sizeof(struct dev_link_t));
> } + if (!link)
> } + return NULL;
> } +
> } link->dev = kmalloc(sizeof(struct dev_node_t), GFP_KERNEL);
> } +
> } + if (!link->dev) {
> } + kfree(link);
> } + return NULL;
> } + }
> } +
> } + memset(link, 0, sizeof(struct dev_link_t));
> } memset(link->dev, 0, sizeof(struct dev_node_t));
> }
> Are you sure this is correct? You are zeroing out the link structure
well spotted, yes, its wrong, thanks for pointing it out.
> (which includes the kmalloc'ed link->dev pointer) and then trying to zero
> out the link->dev pointer. It's a memory leak and NULL pointer reference.
> How about this instead:
>
> link = kmalloc(sizeof(struct dev_link_t), GFP_KERNEL);
>
> if (!link)
> return NULL;
>
> memset(link, 0, sizeof(struct dev_link_t));
> link->dev = kmalloc(sizeof(struct dev_node_t), GFP_KERNEL);
>
> if (!link->dev) {
> kfree(link);
> return NULL;
> }
>
> memset(link->dev, 0, sizeof(struct dev_node_t));
>
> } // dev = init_etherdev(0, sizeof(struct awc_private) );
> ^^ C++ style comments...blech! :)
better, yes, and the comments were not mine, blargh, but I try not to touch
that much, just trying to fix the problems, this is up to the maintainer to
use whatever kind of comments, or el penguino can ask him to change to /**/.
the idea was to just do the menset if the link->dev allocation was successful,
like I did in other drivers in this patch, but yes, coding at 3pm and without
a good amount of caffeine is not safe ;)
- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue Aug 15 2000 - 21:00:16 EST