[PATCH] new applet: nmeter

Denis Vlasenko vda.linux at googlemail.com
Tue Aug 22 08:49:58 UTC 2006


On Monday 21 August 2006 01:04, Rob Landley wrote:
> On Monday 07 August 2006 11:21 am, Denis Vlasenko wrote:
> > Hello Rob,
> > 
> > This is a resend. People raised some concerns/ideas about
> > the applet which were addressed. For example, I did get rid
> > of simple_itoa ;)
> 
> Ok, taking a closer look at this:
> 
> "Released under GPL", could I turn that into our boilerplate line?  (Right now 
> this doesn't specify version, you may remember that came up recently. :)

"Licensed under the GPL v2, see the file LICENSE in this tarball."
Hope this is ok.

> S_STAT(): barf.  You have a macro that defines the first _half_ of a 
> structure, and then you need a closed curly bracket after the macro.  Your 
> macro contains an odd number of curly brackets.  That's really, really ugly.  
> I don't care if the result is more lines of code, please unwrap that.

The problem is not "more lines of code". I need to avoid cut'n'paste errors.
The structures _must_ have same fields at the beginning.
How about having a matched _END macro?

S_STAT(cpu_stat)
       ullong old[CPU_FIELDCNT];
       int bar_sz;
       char *bar;
S_STAT_END(cpu_stat)


> typdef ullong: not good.  Would you rather this was uint64_t, or expanded 
> into "unsigned long long"?  (Query: on 64 bit systems do you want this to be 
> 128 bits?)

I want it to be big enough to hold any /proc value, and then some
(for example, collect_cpu multiplies CPU counts by CPU bar length
at some point in the calculations). "long" is not big enough on i386,
for example, that's why I use long long.

I do not need specifically uint64_t.

typedef is used in order to get type name length down.
"unsigned long long" is just too long to type and read.

> What does vrdval() do?  It requires wrappers for variants of strtoull() in a 
> way that seems like an inline if statement would be simpler...

Comment added, wrappers removed.

> Copying the contents of /proc/meminfo into collect_mem() as a comment doesn't 
> seem helpful, somehow.

Why? Do you want reader to be forced to do cat /proc/meminfo?
(Feel free to shorten that comment, tho... I'm not religious about it).
--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nmeter6.patch
Type: text/x-diff
Size: 22832 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060822/cf66c05f/attachment-0002.bin 


More information about the busybox mailing list