GIF: Split out image descriptor parsing.

1 files changed, 63 insertions, 41 deletions

diff --git a/src/libnsgif.c b/src/libnsgif.c
index e744bcf..054ba9f 100644
--- a/src/libnsgif.c
+++ b/src/libnsgif.c
@@ -51,7 +51,6 @@
 #define GIF_FRAME_RESTORE 3
 #define GIF_FRAME_QUIRKS_RESTORE 4
 
-#define GIF_IMAGE_SEPARATOR 0x2c
 #define GIF_INTERLACE_MASK 0x40
 #define GIF_COLOUR_TABLE_MASK 0x80
 #define GIF_COLOUR_TABLE_SIZE_MASK 0x07
@@ -220,6 +219,59 @@ gif_initialise_frame_extensions(gif_animation *gif, const int frame)
 }
 
 /**
+ * Parse a GIF Image Descriptor.
+ *
+ * The format is:
+ *
+ *  +0   CHAR   Image Separator (0x2c)
+ *  +1   SHORT  Image Left Position
+ *  +3   SHORT  Image Top Position
+ *  +5   SHORT  Width
+ *  +7   SHORT  Height
+ *  +9   CHAR   __Packed Fields__
+ *              1BIT    Local Colour Table Flag
+ *              1BIT    Interlace Flag
+ *              1BIT    Sort Flag
+ *              2BITS   Reserved
+ *              3BITS   Size of Local Colour Table
+ *
+ * \param[in] gif    The gif object we're decoding.
+ * \param[in] frame  The frame to parse an image descriptor for.
+ * \return GIF_OK on success, appropriate error otherwise.
+ */
+static gif_result gif__parse_image_descriptor(
+		struct gif_animation *gif,
+		struct gif_frame *frame)
+{
+	const uint8_t *data = gif->gif_data + gif->buffer_position;
+	size_t len = gif->buffer_size - gif->buffer_position;
+	enum {
+		GIF_IMAGE_DESCRIPTOR_LEN = 10u,
+		GIF_IMAGE_SEPARATOR      = 0x2Cu,
+	};
+
+	assert(gif != NULL);
+	assert(frame != NULL);
+
+	if (len < GIF_IMAGE_DESCRIPTOR_LEN) {
+		return GIF_INSUFFICIENT_FRAME_DATA;
+	}
+
+	if (data[0] != GIF_IMAGE_SEPARATOR) {
+		return GIF_FRAME_DATA_ERROR;
+	}
+
+	frame->redraw_x      = data[1] | (data[2] << 8);
+	frame->redraw_y      = data[3] | (data[4] << 8);
+	frame->redraw_width  = data[5] | (data[6] << 8);
+	frame->redraw_height = data[7] | (data[8] << 8);
+	frame->flags         = data[9];
+
+	gif->buffer_position += GIF_IMAGE_DESCRIPTOR_LEN;
+	return GIF_OK;
+}
+
+/**
  * Attempts to initialise the next frame
  *
  * \param gif The animation context
@@ -314,53 +366,25 @@ static gif_result gif_initialise_frame(gif_animation *gif)
 	gif_data = (gif->gif_data + gif->buffer_position);
 	gif_bytes = (gif_end - gif_data);
 
-	/* Check if we've finished */
-	if (gif_bytes < 1) {
-		return GIF_INSUFFICIENT_FRAME_DATA;
-	}
-
 	if (gif_data[0] == GIF_TRAILER) {
 		gif->buffer_position = (gif_data - gif->gif_data);
 		gif->frame_count = frame + 1;
 		return GIF_OK;
 	}
 
-	/* 10-byte Image Descriptor is:
-	 *
-	 *  +0   CHAR   Image Separator (0x2c)
-	 *  +1   SHORT  Image Left Position
-	 *  +3   SHORT  Image Top Position
-	 *  +5   SHORT  Width
-	 *  +7   SHORT  Height
-	 *  +9   CHAR   __Packed Fields__
-	 *              1BIT    Local Colour Table Flag
-	 *              1BIT    Interlace Flag
-	 *              1BIT    Sort Flag
-	 *              2BITS   Reserved
-	 *              3BITS   Size of Local Colour Table
-	 */
-
-	/* If we're not done, there should be an image descriptor */
-	if (gif_data[0] != GIF_IMAGE_SEPARATOR) {
-		return GIF_FRAME_DATA_ERROR;
+	return_value = gif__parse_image_descriptor(gif, &gif->frames[frame]);
+	if (return_value != GIF_OK) {
+		return return_value;
 	}
 
-	/* Do some simple boundary checking */
-	if (gif_bytes < 10) {
-		return GIF_INSUFFICIENT_FRAME_DATA;
-	}
-	offset_x = gif_data[1] | (gif_data[2] << 8);
-	offset_y = gif_data[3] | (gif_data[4] << 8);
-	width = gif_data[5] | (gif_data[6] << 8);
-	height = gif_data[7] | (gif_data[8] << 8);
-	flags = gif_data[9];
+	gif_data = gif->gif_data + gif->buffer_position;
+	gif_bytes = (gif_end - gif_data);
 
-	/* Set up the redraw area. */
-	gif->frames[frame].redraw_x = offset_x;
-	gif->frames[frame].redraw_y = offset_y;
-	gif->frames[frame].redraw_width = width;
-	gif->frames[frame].redraw_height = height;
-	gif->frames[frame].flags = flags;
+	offset_x = gif->frames[frame].redraw_x;
+	offset_y = gif->frames[frame].redraw_y;
+	width = gif->frames[frame].redraw_width;
+	height = gif->frames[frame].redraw_height;
+	flags = gif->frames[frame].flags;
 
 	/* if we are clearing the background then we need to redraw enough to
 	 * cover the previous frame too
@@ -380,8 +404,6 @@ static gif_result gif_initialise_frame(gif_animation *gif)
 	colour_table_size = 2 << (flags & GIF_COLOUR_TABLE_SIZE_MASK);
 
 	/* Move our data onwards and remember we've got a bit of this frame */
-	gif_data += 10;
-	gif_bytes = (gif_end - gif_data);
 	gif->frame_count_partial = frame + 1;
 
 	/* Skip the local colour table */