[syslinux] Patch sensible callback framework

Ayvaz, James James.Ayvaz at hp.com
Tue May 4 06:45:08 PDT 2010


Is this what you had in mind?  I cleaned up as you suggested and added rarg to hold registration time arguments.  

diff -uprN syslinux-3.86-vanilla/com32/include/syslinux/callback.h syslinux-3.86/com32/include/syslinux/callback.h
--- syslinux-3.86-vanilla/com32/include/syslinux/callback.h	1969-12-31 18:00:00.000000000 -0600
+++ syslinux-3.86/com32/include/syslinux/callback.h	2010-05-04 03:06:31.000000000 -0500
@@ -0,0 +1,25 @@
+#ifndef LIBUTIL_CALLBACK_H
+#define LIBUTIL_CALLBACK_H
+
+#include <stddef.h>
+#include <stdio.h>
+#include <inttypes.h>
+
+struct callback_record;
+
+typedef void (*callback_t)(void *rarg, va_list ap);
+
+typedef struct callback_record  {
+  callback_t function;
+  struct callback_record *prev;
+  struct callback_record *next;
+  void *rarg;
+} callback_record;
+
+
+
+callback_record* register_callback(callback_record **type, callback_t callback, void *rarg);
+int unregister_callback(callback_record **head, callback_record *cb);
+int invoke_callbacks(callback_record **head, ...);
+
+#endif
diff -uprN syslinux-3.86-vanilla/com32/include/syslinux/loadfile.h syslinux-3.86/com32/include/syslinux/loadfile.h
--- syslinux-3.86-vanilla/com32/include/syslinux/loadfile.h	2010-03-31 11:24:25.000000000 -0500
+++ syslinux-3.86/com32/include/syslinux/loadfile.h	2010-04-29 09:42:07.000000000 -0500
@@ -4,10 +4,19 @@
 #include <stddef.h>
 #include <stdio.h>
 
+#include <syslinux/callback.h>
+
 /* loadfile() returns the true size of the file, but will guarantee valid,
    zero-padded memory out to this boundary. */
 #define LOADFILE_ZERO_PAD	64
 
+/*
+ * loadfile callbacks
+ */
+
+extern callback_record *CB_LOADFILE;
+extern callback_record *CB_FLOADFILE;
+
 int loadfile(const char *, void **, size_t *);
 int zloadfile(const char *, void **, size_t *);
 int floadfile(FILE *, void **, size_t *, const void *, size_t);
diff -uprN syslinux-3.86-vanilla/com32/lib/Makefile syslinux-3.86/com32/lib/Makefile
--- syslinux-3.86-vanilla/com32/lib/Makefile	2010-03-31 11:24:25.000000000 -0500
+++ syslinux-3.86/com32/lib/Makefile	2010-04-26 04:57:20.000000000 -0500
@@ -102,6 +102,8 @@ LIBOBJS = \
 	syslinux/run_default.o syslinux/run_command.o			\
 	syslinux/cleanup.o syslinux/localboot.o	syslinux/runimage.o	\
 	\
+	syslinux/callback.o                                             \
+	\
 	syslinux/loadfile.o syslinux/floadfile.o syslinux/zloadfile.o	\
 	\
 	syslinux/load_linux.o syslinux/initramfs.o			\
diff -uprN syslinux-3.86-vanilla/com32/lib/syslinux/callback.c syslinux-3.86/com32/lib/syslinux/callback.c
--- syslinux-3.86-vanilla/com32/lib/syslinux/callback.c	1969-12-31 18:00:00.000000000 -0600
+++ syslinux-3.86/com32/lib/syslinux/callback.c	2010-05-04 03:06:01.000000000 -0500
@@ -0,0 +1,118 @@
+/* ----------------------------------------------------------------------- *
+ *
+ *   Copyright 2005-2008 H. Peter Anvin - All Rights Reserved
+ *
+ *   Permission is hereby granted, free of charge, to any person
+ *   obtaining a copy of this software and associated documentation
+ *   files (the "Software"), to deal in the Software without
+ *   restriction, including without limitation the rights to use,
+ *   copy, modify, merge, publish, distribute, sublicense, and/or
+ *   sell copies of the Software, and to permit persons to whom
+ *   the Software is furnished to do so, subject to the following
+ *   conditions:
+ *
+ *   The above copyright notice and this permission notice shall
+ *   be included in all copies or substantial portions of the Software.
+ *
+ *   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *   EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *   OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *   NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *   HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *   WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *   FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *   OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * ----------------------------------------------------------------------- */
+
+/*
+ * callback.c
+ *
+ * generic callback handlers
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include "syslinux/callback.h"
+
+callback_record* register_callback(callback_record **type, callback_t callback, void *rarg) {
+    callback_record *curr, *new;
+    va_list ap;
+    new = malloc(sizeof(callback_record));
+    if (!new) {
+        return NULL;
+    }
+    new->function = callback;
+    new->next = NULL;
+    new->prev = NULL;
+    new->rarg = rarg;
+
+    if (*type == NULL) {
+        *type = new;
+        return new;
+    }
+ 
+    /* walk to end of the list */
+    curr = *type;
+    while (curr->next)
+        curr = curr->next;
+
+    new->prev = curr;
+    new->next = NULL;
+    curr->next = new;
+    return new;
+}
+
+int unregister_callback(callback_record **head, callback_record *cb) {
+    callback_record *curr;
+    if (!head)
+        return -1;
+    if (!*head)
+        return -1;
+    if (!cb)
+        return -1;
+
+    curr = *head;
+    /* adjust links */
+    if (!cb->prev) {
+        *head = cb->next;
+    }
+    else {
+        cb->prev->next = cb->next;
+    }
+
+    if (cb->next)
+       cb->next->prev = cb->prev;
+
+    free(cb);
+    return 0;
+}
+
+
+int invoke_callbacks(callback_record **head, ...) {
+   va_list ap;
+   callback_record *curr;
+   if (!head) {
+       return -1;
+   }
+   if (!*head) {
+       return -1;
+   }
+
+   curr = *head;
+   do {
+       if (curr->function) {
+           va_start(ap, head);
+           curr->function(curr->rarg, ap);
+           va_end(ap);
+       }
+
+       curr = curr->next;
+   } while(curr);
+}
+
diff -uprN syslinux-3.86-vanilla/com32/lib/syslinux/floadfile.c syslinux-3.86/com32/lib/syslinux/floadfile.c
--- syslinux-3.86-vanilla/com32/lib/syslinux/floadfile.c	2010-03-31 11:24:25.000000000 -0500
+++ syslinux-3.86/com32/lib/syslinux/floadfile.c	2010-05-03 10:05:30.000000000 -0500
@@ -38,10 +38,13 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 
+#include <syslinux/callback.h>
 #include <syslinux/loadfile.h>
 
 #define INCREMENTAL_CHUNK 1024*1024
 
+callback_record *CB_FLOADFILE = NULL;
+
 int floadfile(FILE * f, void **ptr, size_t * len, const void *prefix,
 	      size_t prefix_len)
 {
@@ -54,7 +57,6 @@ int floadfile(FILE * f, void **ptr, size
 
     if (fstat(fileno(f), &st))
 	goto err;
-
     if (!S_ISREG(st.st_mode)) {
 	/* Not a regular file, we can't assume we know the file size */
 	if (prefix_len) {
@@ -75,6 +77,10 @@ int floadfile(FILE * f, void **ptr, size
 
 	    rlen = fread((char *)data + clen, 1, alen - clen, f);
 	    clen += rlen;
+
+            if (CB_FLOADFILE) {
+                invoke_callbacks(&CB_FLOADFILE, clen, -1);
+            }
 	} while (clen == alen);
 
 	*len = clen;
@@ -93,11 +99,29 @@ int floadfile(FILE * f, void **ptr, size
 
 	memcpy(data, prefix, prefix_len);
 
-	if ((off_t) fread((char *)data + prefix_len, 1, clen - prefix_len, f)
-	    != clen - prefix_len)
-	    goto err;
+        /* a callback is registered so read file in chunks */
+        if (CB_FLOADFILE) {
+	    clen = alen = prefix_len;
+            do {
+	        alen += INCREMENTAL_CHUNK;
+	        dp = realloc(data, alen);
+	        if (!dp)
+		    goto err;
+	        data = dp;
+
+	        rlen = fread((char *)data + clen, 1, alen - clen, f);
+	        clen += rlen;
+	        invoke_callbacks(&CB_FLOADFILE, (clen + LOADFILE_ZERO_PAD - 1) & ~(LOADFILE_ZERO_PAD - 1), xlen);
+	    } while (clen == alen);
+	    *ptr = data;
+        }
+        /* read whole file at once */
+        else {
+            if ((off_t) fread((char *)data + prefix_len, 1, clen - prefix_len, f)
+	        != clen - prefix_len)
+                goto err;
+        }
     }
-
     memset((char *)data + clen, 0, xlen - clen);
     return 0;
 
diff -uprN syslinux-3.86-vanilla/com32/lib/syslinux/loadfile.c syslinux-3.86/com32/lib/syslinux/loadfile.c
--- syslinux-3.86-vanilla/com32/lib/syslinux/loadfile.c	2010-03-31 11:24:25.000000000 -0500
+++ syslinux-3.86/com32/lib/syslinux/loadfile.c	2010-05-04 03:13:13.000000000 -0500
@@ -39,10 +39,21 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 
+#include <syslinux/callback.h>
 #include <syslinux/loadfile.h>
 
 #define INCREMENTAL_CHUNK 1024*1024
 
+callback_record *CB_LOADFILE;
+
+void loadfile_cb(void *rarg, va_list ap) {
+    char *filename = (char*)rarg;
+    size_t cur = va_arg(ap, size_t);
+    size_t total = va_arg(ap, size_t);
+    invoke_callbacks(&CB_LOADFILE, filename, cur, total);
+}
+
+
 int loadfile(const char *filename, void **ptr, size_t * len)
 {
     FILE *f;
@@ -52,7 +63,21 @@ int loadfile(const char *filename, void
     if (!f)
 	return -1;
 
+    /*
+     * if a CB_LOADFILE callback is registered, let's register a CB_FLOADFILE callback
+     * so that we can add the filename
+     */
+    callback_record *cb = NULL;
+    if (CB_LOADFILE) {
+        cb = register_callback(&CB_FLOADFILE, &loadfile_cb, filename);
+    }
     rv = floadfile(f, ptr, len, NULL, 0);
+
+    /* unregister our callback */
+    if (cb) {
+        unregister_callback(&CB_FLOADFILE, cb);
+    }
+
     e = errno;
 
     fclose(f);


-----Original Message-----
From: H. Peter Anvin [mailto:hpa at zytor.com] 
Sent: Monday, May 03, 2010 1:10 PM
To: syslinux at zytor.com
Cc: Ayvaz, James
Subject: Re: [syslinux] Patch sensible callback framework

On 05/03/2010 09:20 AM, Ayvaz, James wrote:
> Resubmit with changes from Sebastian Herbszt
> +
> +callback_record* register_callback(callback_record **type, callback_t callback) {
> +    callback_record *curr, *new;
> +    if (type == NULL)
> +        return NULL;
> +    if (*type == NULL) {
> +        /* type is the head of the list */
> +        new = malloc(sizeof(callback_record));
> +        if (!new) {
> +            return NULL;
> +        }
> +        memset(new, 0, sizeof(callback_record));
> +        new->function = callback;
> +        new->prev = NULL;
> +        new->next = NULL;
> +        *type = new;
> +        return *type;
> +    }
> +    else {
> +        /* check to see if we're already registered */
> +        curr = *type;
> +        while (curr->next) {
> +            if (curr->function == callback) {
> +                return curr;
> +            }
> +            curr = curr->next;
> +        }
> +        /* add new callback entry to list */
> +        new = malloc(sizeof(callback_record));
> +        if (!new) {
> +            return NULL;
> +        }
> +
> +        memset(new, 0, sizeof(callback_record));
> +        new->function = callback;
> +        new->prev = curr;
> +        new->next = NULL;
> +        curr->next = new;
> +        return new;
> +    }
> +}


This is *way* more complex than it need to be.

First of all, I don't see much of a point to have the registration
system prevent multiple registrations, or to enforce a specific order.
Second, there is no reason to have two branches.  Third, there is no
point in using memset() since we will fill in every record in the
structure.  If there *was*, you should use calloc() or zalloc(), not
malloc+memset.

Also, checking for (type == NULL) isn't really meaningful... it's not a
very likely error and the error handling in the caller is likely to be
nonexistent.

If you *do* prevent multiple registrations, you probably would need a
registration counter which is decremented at unregister time so the
sequence register-register-unregister-unregister doesn't crash the
system, but really the better thing to do is probably to just allow the
double registration.

I still want to be able to pass a user argument to the callback, too.

It would be nice if you could submit the callback framework as a
separate patch from the actual usages of the framework.

	-hpa




More information about the Syslinux mailing list