What is the verdict on these? My stack of local patches is getting uncomfortably long, so decisions either way are welcome so I can shrink it. Regards, Jeremy Henty On Mon, May 04, 2009 at 06:59:34AM +0100, Jeremy Henty wrote:
Three more cleanup patches:
1: Inline Cache_client_make_key() into Cache_client_enqueue(). Why have a separate function just to increment a variable? It's just as clear (and less noisy) to increment a static variable inside Cache_client_enqueue().
2: The width and height arguments of a_Image_new() do nothing. The function itself ignores them and its callers always set them to 0. Since neither the function or its callers seem to care what these arguments are, why not remove them?
3: a_Web_dispatch_by_type() has a strange convention for its return type, it returns -1 to indicate failure. It's less confusing to use the common convention of a boolean: 1 indicates success and 0 indicates failure.
Regards,
Jeremy Henty
diff -r 817ab0ff08a1 -r c671a697420f src/cache.c --- a/src/cache.c Sat May 02 22:57:36 2009 +0100 +++ b/src/cache.c Sat May 02 23:27:14 2009 +0100 @@ -136,18 +136,6 @@ /* Client operations ------------------------------------------------------ */
/* - * Make a unique primary-key for cache clients - */ -static int Cache_client_make_key(void) -{ - static int ClientKey = 0; /* Provide a primary key for each client */ - - if (++ClientKey <= 0) - ClientKey = 1; - return ClientKey; -} - -/* * Add a client to ClientQueue. * - Every client-field is just a reference (except 'Web'). * - Return a unique number for identifying the client. @@ -155,11 +143,13 @@ static int Cache_client_enqueue(const DilloUrl *Url, DilloWeb *Web, CA_Callback_t Callback, void *CbData) { - int ClientKey; + static int ClientKey = 0; /* Provide a primary key for each client */ CacheClient_t *NewClient;
+ if (++ClientKey <= 0) + ClientKey = 1; + NewClient = dNew(CacheClient_t, 1); - ClientKey = Cache_client_make_key(); NewClient->Key = ClientKey; NewClient->Url = Url; NewClient->Version = 0;
diff -r 4ab60f74c46b src/dicache.c --- a/src/dicache.c Sun May 03 15:29:42 2009 +0100 +++ b/src/dicache.c Sun May 03 15:32:34 2009 +0100 @@ -398,7 +398,7 @@ dReturn_val_if_fail(MimeType && Ptr, NULL);
if (!web->Image) - web->Image = a_Image_new(0, 0, NULL, web->bgColor); + web->Image = a_Image_new(NULL, web->bgColor);
/* Add an extra reference to the Image (for dicache usage) */ a_Image_ref(web->Image); diff -r 4ab60f74c46b src/html.cc --- a/src/html.cc Sun May 03 15:29:42 2009 +0100 +++ b/src/html.cc Sun May 03 15:32:34 2009 +0100 @@ -2062,7 +2062,7 @@ html->styleEngine->setNonCssHints(&props);
/* Add a new image widget to this page */ - Image = a_Image_new(0, 0, alt_ptr, 0); + Image = a_Image_new(alt_ptr, 0); if (DW2TB(html->dw)->getBgColor()) Image->bg_color = DW2TB(html->dw)->getBgColor()->getColor();
diff -r 4ab60f74c46b src/image.cc --- a/src/image.cc Sun May 03 15:29:42 2009 +0100 +++ b/src/image.cc Sun May 03 15:32:34 2009 +0100 @@ -33,10 +33,7 @@ /* * Create and initialize a new image structure. */ -DilloImage *a_Image_new(int width, - int height, - const char *alt_text, - int32_t bg_color) +DilloImage *a_Image_new(const char *alt_text, int32_t bg_color) { DilloImage *Image;
diff -r 4ab60f74c46b src/image.hh --- a/src/image.hh Sun May 03 15:29:42 2009 +0100 +++ b/src/image.hh Sun May 03 15:32:34 2009 +0100 @@ -50,8 +50,7 @@ /* * Function prototypes */ -DilloImage *a_Image_new(int width, int height, - const char *alt_text, int32_t bg_color); +DilloImage *a_Image_new(const char *alt_text, int32_t bg_color); void a_Image_ref(DilloImage *Image); void a_Image_unref(DilloImage *Image);
diff -r 02d822ae435a src/cache.c --- a/src/cache.c Sat May 02 23:38:41 2009 +0100 +++ b/src/cache.c Sat May 02 23:42:40 2009 +0100 @@ -1112,7 +1112,7 @@ const char *curr_type = Cache_current_content_type(entry); st = a_Web_dispatch_by_type(curr_type, ClientWeb, &Client->Callback, &Client->CbData); - if (st == -1) { + if (!st) { /* MIME type is not viewable */ if (ClientWeb->flags & WEB_RootUrl) { MSG("Content-Type '%s' not viewable.\n", curr_type); diff -r 02d822ae435a src/web.cc --- a/src/web.cc Sat May 02 23:38:41 2009 +0100 +++ b/src/web.cc Sat May 02 23:42:40 2009 +0100 @@ -47,8 +47,8 @@ * Given the MIME content type, and a fd to read it from, * this function connects the proper MIME viewer to it. * Return value: - * 1 on success (and Call and Data properly set). - * -1 for unhandled MIME types (and Call and Data untouched). + * 1 on success (and Call and Data properly set). + * 0 for unhandled MIME types (and Call and Data untouched). */ int a_Web_dispatch_by_type (const char *Type, DilloWeb *Web, CA_Callback_t *Call, void **Data) @@ -57,7 +57,7 @@
_MSG("a_Web_dispatch_by_type\n");
- dReturn_val_if_fail(Web->bw != NULL, -1); + dReturn_val_if_fail(Web->bw != NULL, 0);
// get the Layout object from the bw structure. Layout *layout = (Layout*)Web->bw->render_layout; @@ -71,8 +71,8 @@ Web->bgColor= styleEngine.backgroundStyle()->backgroundColor->getColor();
dw = (Widget*) a_Mime_set_viewer(Type, Web, Call, Data); - if (dw == NULL) - return -1; + if (!dw) + return 0;
dw->setStyle (styleEngine.style ());
@@ -100,7 +100,7 @@ if (!dw) { MSG_HTTP("unhandled MIME type: \"%s\"\n", Type); } - return (dw ? 1 : -1); + return dw ? 1 : 0; }
diff -r 02d822ae435a src/web.hh --- a/src/web.hh Sat May 02 23:38:41 2009 +0100 +++ b/src/web.hh Sat May 02 23:42:40 2009 +0100 @@ -38,7 +38,7 @@ int a_Web_valid(DilloWeb *web); void a_Web_free (DilloWeb*); int a_Web_dispatch_by_type (const char *Type, DilloWeb *web, - CA_Callback_t *Call, void **Data); + CA_Callback_t *Call, void **Data);
#ifdef __cplusplus }
_______________________________________________ Dillo-dev mailing list Dillo-dev@dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev