[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [BusyBox] vlanconfig - alternative to vconfig applet
From:       Nick Fedchik <fnm () fusion ! ukrsat ! com>
Date:       2003-01-28 5:48:04
[Download RAW message or body]

On Mon, 27 Jan 2003, Manuel Novoa III wrote:
>On Sun, Jan 26, 2003 at 05:48:35PM +0200, Nick Fedchik wrote:
>> On Sat, 25 Jan 2003, Nick Fedchik wrote:
>> ...
>> > so much more changes and have made a decision to make new
>> > applet - vlanconfig.
>While I commend your motivation and efforts, I suspect that anyone
>using vconfig (I don't btw) isn't likely to want to change their
>scripts, etc. to use your different command line args.
Yes, I sure when different command line args isn't likely to someone...
That's why I rename vconfig to vlanconfig.
But cmd line set is not a principal.
I only don't like "rem" command - "del" is widely used.

>  text    data     bss     dec     hex filename
>  2835       0       0    2835     b13 vconfig-dist.o  (current vconfig)
>  1089       0       0    1089     441 vlanconfig.o    (your alternate)
>   798       0       0     798     31e vconfig.o       (my vconfig)
Excellent!

>As I said, I don't use vconfig myself, so I only tested that the
>arg processing was correct.
I shall test it in the near future.

>I hope you and others find the following comments and code example useful.
Sure.

>-------------------------------------------------------------------
>(your applet... minus headers)
...
>It may generate smaller code if the socket call/fd assignment is closer
>to the ioctl call.
Yes, but it's nonsense to do all the checks if no socket interface
supported.
That's why I put it at begin of code.

>Since we're guaranteed to go through the loop at least once, it is
>sometimes smaller to use a do {} while, depending on the test, etc.
>Your mileage may vary but it is worth looking at.
Thank, I shall pay to this my attention in future.
I haven't so good skills in the programming, but I hope when it's better
than nothing to do. :)
Also I very hurry up to port a neede for me apps to BB. I sure when it's not
good, f.e. my first buggy vlanconfig patch :(

>By creating a "set" arg prefixing the nametype/flag/egress/ingress args,
>you've made it necessary to use a second loop.  Consider removing "set".
I agree, see my comments about CLI above.

>The argc test, strtoul call, range check is repeated code.  In my
>version, the needed argc is stored in the command table and I only
>need to check twice... once to make sure there are at least 3 args,
>and then once when the command is determined.
>I also use a static function that does the conversion and range check
>to consolidate things.
Overlooked it. IMHO that xstrtoul10() should be in the libbb or into
another lib.
F.e. device name check is freq. task.

>I also store the ifr.cmd value in the command table.  Hence I only need
>one 'ifr.cmd = <value>;' statment.
I've the same idea, but not realized it at first stage.

Generally I very satisfied by given results!
Now it has no reason to include my vlanconfig to the BusyBox code, let it be
this vconfig applet.
I think we may inform to Ben Greear about it after a few rollout of vconfig
applet code.

Now I try to port brcfg to BB, so I hope about help to do that.

-- 
Nick Fedchik, FNM3-RIPE(UANIC)
Internet Dept./ISP UkrSat, Kiev, Ukraine

[prev in list] [next in list] [prev in thread] [next in thread]