> #ifndef MODULE
> dev=init_trdev(dev, 0);
> +
> + if (!dev)
> + goto fail_alloc_dev;
> #endif
Not needed, this never fails in the driver, dev is already allocated and
having no dev structure is trapped in the appropriate places, and we don't
ask the function to allocate any private memory either. If you had
completely read the code you would have realised that the init_trdev call
never fails for the driver.
> - if(olympic_init(dev)==-1) {
> - unregister_netdevice(dev);
> - kfree(dev->priv);
> - return 0;
> - }
> + if(olympic_init(dev)==-1)
> + goto fail_init;
>
> dev->open=&olympic_open;
> dev->hard_start_xmit=&olympic_xmit;
>@@ -249,6 +257,13 @@
> }
> }
> return 0 ;
>+
>+fail_init:
>+ unregister_netdevice(dev);
>+ kfree(dev);
>+fail_alloc_dev:
>+ kfree(olympic_priv);
>+ return 0;
> }
Why ??, you have simply moved the same lines to a different place in the
driver, and as said above fail_alloc_dev will never get called and I dislike
goto's in the code anyway. Even if the same couple of lines are repeated in
the code you have to look at the whole picture, i.e. where are the lines
called ? if in the init functions they are only called once and saving this
minimal amount of space is not necessary, especially if it hurts the
readability of the code. Also you need to take a look at the assembly
produced by the driver to see if the compiler is being intelligent for us
and alleviating the issue anyway.
As mentioned in my earlier mail replying to Jeff, there are a lot of updates
required in the driver to make it use the new pci api properly and to fix
some other issues. I will do these changes and they will negate the
necessity for some of the changes you propose anyway. Please don't make
changes to the code unless they really do make life a lot easier or fix some
unnecessary bug. Changing code just because you have a certain preference
for coding style or to save one or two lines of source code is not a good
thing (there are only a few people who have this power over Linux, guess who
?).
Mike Phillips
-
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:23 EST