[BusyBox] [patch] New applet, "count".

Manuel Novoa III mjn3 at codepoet.org
Sat Oct 4 01:29:37 UTC 2003


Hello Rob,

On Fri, Oct 03, 2003 at 12:33:37AM -0500, Rob Landley wrote:
> This is something I've used a lot, it's a quick and dirty progress indicator 

I'm afraid the emphasis seems to be on "dirty".  There are a number
of problems, as explained below.

> #include <stdio.h>
> #include <unistd.h>
> #include "busybox.h"
> 
> /* Pipes work in system page size chunks. */
> #define BUFFER_SIZE 4096
> char *message="%ld bytes%c";

static const char message[] = "%ld bytes%c";

would have been better here for both size and namespace reasons.
But you don't really need to reference message twice (see below).

In any case, it is always advisable to run both 'size' and 'nm'
on an applet's object file to look for potential problems.

> 
> int count_main(int argc, char **argv)
> {
>     long bytes=0;

Just as easy to use an unsigned long here, and it might be worth making
using unsigned long long an option since you don't know how much data
might be passing through.

>     char buf[BUFFER_SIZE];

Busybox has configureable buffer allocation policies.  See busybox.h
for RESERVE_CONFIG_BUFFER and RELEASE_CONFIG_BUFFER defines.  Some
environments (notably uClinux) have limited stack space.

> 
>     for(;;) {
>         int len=read(0,buf,BUFSIZ);

You want BUFFER_SIZE here.

>         if(len) {
>             bytes+=len;
>             write(1,buf,len);

And if read fails, or write fails or is short?  There really should be
some error checking.  Otherwise, using this in an archiving pipeline
is dangerous.

>             fprintf(stderr,message,bytes,'\r');
>         } else break;
>     }
> 
>     fprintf(stderr,message,bytes,'\n');

You can replace the two fprintf calls with one fprintf and one fputc,
as illustrated below.

> 
>     return 0;
> }

Consider the rewritten version below.  It is slightly larger, but does
proper error checking and has better busybox integration.

Manuel



#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include "busybox.h"

#define BUFFER_SIZE 4096
                                                                                
int count_main(int argc, char **argv)
{
    /* Do we want an unsigned long long counter? */
#if 0
    static const char fmt[] = "\r%llu bytes";
    unsigned long long bytes = 0;
#else
    static const char fmt[] = "\r%lu bytes";
    unsigned long bytes = 0;
#endif
    ssize_t rv;
    RESERVE_CONFIG_BUFFER(buf, BUFFER_SIZE);

    do {
        /* Note: bb_full_write is safe for size==0, unlike write(). */
        if (((rv = bb_safe_read(STDIN_FILENO, buf, BUFFER_SIZE)) >= 0)
            && (bb_full_write(STDOUT_FILENO, buf, rv) >= 0)
            ) {
            bb_fprintf(stderr, fmt, (bytes += rv));
        }
    } while (rv > 0);

    RELEASE_CONFIG_BUFFER(buf);

    if (rv == 0) {                /* EOF, so successful. */
        fputc('\n', stderr);
        /* To check for output errors to stderr, uncomment the next line.
         * Doing so is probably not desired for this applet though, since
         * the counts are only advisory. */
        /* if (!ferror(stderr)) */
        return EXIT_SUCCESS;
    }

    /* Either rv < 0 (read failed), or write failed (or ferror(stderr)). */
    bb_perror_msg_and_die("i/o");
}




More information about the busybox mailing list