[syslinux] Patch sensible callback framework

[Sebastian Herbszt]

Ayvaz, James wrote:

[snip]

> diff -uprN syslinux-3.86-vanilla/com32/modules/linux.c syslinux-3.86/com32/modules/linux.c
> --- syslinux-3.86-vanilla/com32/modules/linux.c 2010-03-31 11:24:25.000000000 -0500
> +++ syslinux-3.86/com32/modules/linux.c 2010-04-29 10:17:19.000000000 -0500
> @@ -43,11 +43,13 @@
> #include <stdio.h>
> #include <string.h>
> #include <console.h>
> +#include <syslinux/callback.h>
> #include <syslinux/loadfile.h>
> #include <syslinux/linux.h>
> #include <syslinux/pxe.h>
> 
> const char *progname = "linux.c32";
> +callback_record *CB_LOADFILE = NULL;
> 
> /* Find the last instance of a particular command line argument
>    (which should include the final =; do not use for boolean arguments) */
> @@ -108,6 +110,53 @@ static char *make_cmdline(char **argv)
>     return cmdline;
> }
> 
> +void linux_percent_progress_cb(va_list ap) {
> +    char *file     = va_arg(ap, char*);
> +    size_t cur     = va_arg(ap, size_t);
> +    size_t total   = va_arg(ap, size_t);
> +    int percent    = 0;
> +
> +    if (total > 0) {
> +        percent = (int)(((float)cur / (float)total) * 100.0);
> +    }
> +    clear_line();
> +    move_cursor_to_column(0);
> +    printf("Loading: %s %d%%", file, percent);

Strip the colon here - it's not used below.

> +}
> +
> +void linux_dot_progress_cb(va_list ap) {
> +    char *file     = va_arg(ap, char*);
> +    size_t cur     = va_arg(ap, size_t);
> +    size_t total   = va_arg(ap, size_t);
> +    int percent    = 0;
> +    int i          = 0;
> +    if (total > 0) {
> +        percent = (int)(((float)cur / (float)total) * 100.0);
> +    }
> +
> +    clear_line();
> +    move_cursor_to_column(0);
> +
> +    printf("Loading %s", file);
> +    while(i < percent) {
> +        printf(".");
> +        i += 5;
> +    }
> +}
> +
> +void linux_done_progress_cb(va_list ap) {
> +    char *file     = va_arg(ap, char*);
> +    size_t cur     = va_arg(ap, size_t);
> +    size_t total   = va_arg(ap, size_t);
> +    int percent    = 0;
> +    if (total > 0) {
> +        percent = (int)(((float)cur / (float)total) * 100.0);
> +    }
> +
> +    if (percent >= 100)
> +        printf(" OK\n");
> +}

I think "total" can never be 0, because syslinux handles 0 byte files as non existent.
If that is correct "total" can be > 0 or -1 for unknown size. In the latter case "OK" is never printed.
That printf() can be put after the calls to loadfile() and initramfs_load_archive() if those succeed and
opt_quiet is false. This callback can then be removed.

> +
> int main(int argc, char *argv[])
> {
>     const char *kernel_name;
> @@ -118,11 +167,14 @@ int main(int argc, char *argv[])
>     size_t kernel_len;
>     bool opt_dhcpinfo = false;
>     bool opt_quiet = false;
> +    bool opt_percent = false;
>     void *dhcpdata;
>     size_t dhcplen;
>     char **argp, *arg, *p;
> +    callback_record *progress_cb, *complete_cb;
> +
> 
> -    openconsole(&dev_null_r, &dev_stdcon_w);
> +    console_ansi_raw();
> 
>     (void)argc;
>     argp = argv + 1;
> @@ -130,7 +182,11 @@ int main(int argc, char *argv[])
>     while ((arg = *argp) && arg[0] == '-') {
>        if (!strcmp("-dhcpinfo", arg)) {
>            opt_dhcpinfo = true;
> +       }
> +       else if (!strcmp("-percent", arg)) {
> +           opt_percent = true;
>        } else {
> +

Strip this introduced new line.

>            fprintf(stderr, "%s: unknown option: %s\n", progname, arg);
>            return 1;
>        }
> @@ -157,16 +213,21 @@ int main(int argc, char *argv[])
>     if (find_boolean(argp, "quiet"))
>        opt_quiet = true;
> 
> -    if (!opt_quiet)
> -       printf("Loading %s... ", kernel_name);
> +    if (!opt_quiet) {

A printf("Loading %s ", kernel_name); is needed here in case loadfile() fails and opt_quiet is false.
Else we just get a line with "failed!".

> +        if (opt_percent) {
> +            progress_cb = register_callback(&CB_LOADFILE, &linux_percent_progress_cb);
> +        }
> +        else {
> +            progress_cb = register_callback(&CB_LOADFILE, &linux_dot_progress_cb);
> +        }
> +        complete_cb = register_callback(&CB_LOADFILE, &linux_done_progress_cb);
> +    }
>     if (loadfile(kernel_name, &kernel_data, &kernel_len)) {
>        if (opt_quiet)
>            printf("Loading %s ", kernel_name);
>        printf("failed!\n");
>        goto bail;
>     }
> -    if (!opt_quiet)
> -       printf("ok\n");
> 
>     cmdline = make_cmdline(argp);
>     if (!cmdline)
> @@ -183,22 +244,23 @@ int main(int argc, char *argv[])
>            if (p)
>                *p = '\0';
> 
> -           if (!opt_quiet)
> -               printf("Loading %s... ", arg);
>            if (initramfs_load_archive(initramfs, arg)) {
>                if (opt_quiet)
>                    printf("Loading %s ", kernel_name);
>                printf("failed!\n");
>                goto bail;
>            }

printf("Loading %s ", arg);  should be kept here before initramfs_load_archive() if opt_quiet is false.

> -           if (!opt_quiet)
> -               printf("ok\n");
> 
>            if (p)
>                *p++ = ',';
>        } while ((arg = p));
>     }
> 
> +    if (!opt_quiet) {
> +        unregister_callback(&CB_LOADFILE, progress_cb);
> +        unregister_callback(&CB_LOADFILE, complete_cb);
> +    }
> +
>     /* Append the DHCP info */
>     if (opt_dhcpinfo &&
>        !pxe_get_cached_info(PXENV_PACKET_TYPE_DHCP_ACK, &dhcpdata, &dhcplen)) {
> 

Sebastian