From 594857c92c44f1f1c3acf3d807ef3e91b89c911b Mon Sep 17 00:00:00 2001
From: Gareth Evans <gareth.evans@contextis.co.uk>
Date: Fri, 3 Feb 2017 12:52:15 +0000
Subject: [PATCH] Various maintainability and speed improvements

---
 image-ppm.hh |  4 +++-
 image.cc     | 31 ++++++++++++++++++++++++-------
 image.hh     | 12 ++++++++----
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/image-ppm.hh b/image-ppm.hh
index dfe007b..fe40ede 100644
--- a/image-ppm.hh
+++ b/image-ppm.hh
@@ -14,8 +14,10 @@ int ppm_write(image_ctx_t *ctx, FILE* fd);
 image_dispatch_table_t ppm_dt = {\
 	.init 		= ppm_init, \
 	.add_pixel 	= ppm_add_pixel, \
+	.set_pixel	= NULL, \
 	.get_pixel	= ppm_get_pixel, \
-	.write		= ppm_write
+	.write		= ppm_write, \
+	.free		= NULL
 };
 
 #endif
\ No newline at end of file
diff --git a/image.cc b/image.cc
index c8e2098..1af4e72 100644
--- a/image.cc
+++ b/image.cc
@@ -17,6 +17,8 @@
 int get_dt(image_dispatch_table_t *dt, int format);
 int load_library(image_dispatch_table_t *dt);
 
+#define DISPATCH_TABLE(ctx) ((image_dispatch_table_t*)(ctx)->_dt)
+
 static int default_format = IMAGE_PPM;
 char *dynamic_backend = NULL;
 
@@ -101,26 +103,35 @@ int image_init(image_ctx_t *ctx, \
  */
 int image_add_pixel(image_ctx_t *ctx, const uint8_t r, const uint8_t g, const uint8_t b, const uint8_t a){
 	if(ctx->initialized != 1) return EINVAL;
-	return ((image_dispatch_table_t*)ctx->_dt)->add_pixel(ctx,r,g,b,a);
+	return DISPATCH_TABLE(ctx)->add_pixel(ctx,r,g,b,a);
 }
 int image_set_pixel(image_ctx_t *ctx, const size_t x, const size_t y, const uint8_t r, const uint8_t g, const uint8_t b, const uint8_t a){
 	size_t block_size;
 	if(ctx->initialized != 1) return EINVAL;
+	/* Image format handlers have the option to specify a _set_pixel handler */
+	if(DISPATCH_TABLE(ctx)->set_pixel != NULL){
+		return DISPATCH_TABLE(ctx)->set_pixel(ctx,x,y,r,g,b,a);
+	}
 	block_size = ctx->model == IMAGE_RGB ? RGB_SIZE : RGBA_SIZE;
 	ctx->next_pixel = ctx->canvas + PIXEL_OFFSET(x,y,ctx->width,block_size);
-	return ((image_dispatch_table_t*)ctx->_dt)->add_pixel(ctx,r,g,b,a);
+	return DISPATCH_TABLE(ctx)->add_pixel(ctx,r,g,b,a);
 }
 int image_get_pixel(image_ctx_t *ctx, const size_t x, const size_t y, const uint8_t *r, const uint8_t *g, const uint8_t *b, const uint8_t *a){
 	if(ctx->initialized != 1) return EINVAL;
-	return ((image_dispatch_table_t*)ctx->_dt)->get_pixel(ctx,x,y,r,g,b,a);
+	if(DISPATCH_TABLE(ctx)->get_pixel != NULL)
+		return DISPATCH_TABLE(ctx)->get_pixel(ctx,x,y,r,g,b,a);
+	return ENOSYS;
 }
 int image_write(image_ctx_t *ctx, FILE *fd){
 	if(ctx->initialized != 1) return EINVAL;
-	return ((image_dispatch_table_t*)ctx->_dt)->write(ctx,fd);
+	return DISPATCH_TABLE(ctx)->write(ctx,fd);
 }
 void image_free(image_ctx_t *ctx){
 	if(ctx->initialized != 1) return;
-	free(ctx->canvas);
+	if(DISPATCH_TABLE(ctx)->free != NULL){
+		DISPATCH_TABLE(ctx)->free(ctx);
+	}
+	if(ctx->canvas != NULL) free(ctx->canvas);
 }
 
 /*
@@ -248,10 +259,16 @@ int load_library(image_dispatch_table_t *dt){
 	/* Perform symbol lookup */
 	if((dt->init = (_init*)dlsym(hndl,"lib_init")) == NULL ||
 		(dt->add_pixel = (_add_pixel*)dlsym(hndl,"lib_add_pixel")) == NULL ||
-		(dt->get_pixel = (_get_pixel*)dlsym(hndl,"lib_get_pixel")) == NULL ||
 		(dt->write = (_write*)dlsym(hndl,"lib_write")) == NULL){
 		fprintf(stderr,"Invalid image processing module specified\n\t%s",dlerror());
-		success = dlclose(hndl);
+		success = EINVAL;
+		(void) dlclose(hndl);
+	}
+	/* Lookup optional symbols, these can return NULL */
+	if(success == 0){
+		dt->get_pixel = (_get_pixel*)dlsym(hndl,"lib_get_pixel");
+		dt->set_pixel = (_set_pixel*)dlsym(hndl,"lib_set_pixel");
+		dt->free = (_free*)dlsym(hndl,"lib_free");
 	}
 
 	return success;
diff --git a/image.hh b/image.hh
index fa456ef..537b053 100644
--- a/image.hh
+++ b/image.hh
@@ -31,14 +31,18 @@ typedef struct _image_ctx{
 
 typedef int _init(image_ctx_t*);
 typedef int _add_pixel(image_ctx_t*,const uint8_t,const uint8_t,const uint8_t,const uint8_t);
+typedef int _set_pixel(image_ctx_t*,const size_t,const size_t,const uint8_t,const uint8_t,const uint8_t,const uint8_t);
 typedef int _get_pixel(image_ctx_t*,const size_t,const size_t,const uint8_t*,const uint8_t*,const uint8_t*,const uint8_t*);
 typedef int _write(image_ctx_t*,FILE*);
+typedef void _free(image_ctx_t*);
 
 typedef struct _image_dispatch_table{
-	_init *init;
-	_add_pixel *add_pixel;
-	_get_pixel *get_pixel;
-	_write *write;
+	_init		*init;
+	_add_pixel	*add_pixel;
+	_set_pixel	*set_pixel;
+	_get_pixel	*get_pixel;
+	_write		*write;
+	_free		*free;
 } image_dispatch_table_t;
 
 int image_set_format(int);