Add additional error checking to helpers.

core/compress/common.odin

@@ -11,6 +11,7 @@ package compress
 import "core:io"
 import "core:image"
 import "core:bytes"
+import "core:mem"
 
 /*
 	These settings bound how much compression algorithms will allocate for their output buffer.
@@ -47,6 +48,7 @@ when size_of(uintptr) == 8 {
 
 Error :: union {
 	General_Error,
+	mem.Allocator_Error,
 	Deflate_Error,
 	ZLIB_Error,
 	GZIP_Error,

core/image/common.odin

@@ -131,6 +131,7 @@ Error :: enum {
 	Unknown_Interlace_Method,
 	Requested_Channel_Not_Present,
 	Post_Processing_Error,
+	Invalid_Chunk_Length,
 }
 
 /*

core/image/png/example.odin

@@ -41,7 +41,7 @@ main :: proc() {
 demo :: proc() {
 	file: string
 
-	options := image.Options{} // {.return_metadata};
+	options := image.Options{.return_metadata}
 	err:       compress.Error
 	img:      ^image.Image
 
@@ -53,23 +53,26 @@ demo :: proc() {
 	if err != nil {
 		fmt.printf("Trying to read PNG file %v returned %v\n", file, err)
 	} else {
-		v: ^Info
-
 		fmt.printf("Image: %vx%vx%v, %v-bit.\n", img.width, img.height, img.channels, img.depth)
-		if img.metadata_ptr != nil && img.metadata_type == Info {
-			v = (^Info)(img.metadata_ptr)
 
-			// Handle ancillary chunks as you wish.
+		assert(img.metadata_ptr != nil && img.metadata_type == Info)
-			// We provide helper functions for a few types.
+
-			for c in v.chunks {
+		v := (^Info)(img.metadata_ptr)
-				#partial switch c.header.type {
+
-				case .tIME:
+		// Handle ancillary chunks as you wish.
-					t, _ := core_time(c)
+		// We provide helper functions for a few types.
+		for c in v.chunks {
+			#partial switch c.header.type {
+			case .tIME:
+				if t, t_ok := core_time(c); t_ok {
 					fmt.printf("[tIME]: %v\n", t)
-				case .gAMA:
+				}
-					fmt.printf("[gAMA]: %v\n", gamma(c))
+			case .gAMA:
-				case .pHYs:
+				if gama, gama_ok := gamma(c); gama_ok {
-					phys := phys(c)
+					fmt.printf("[gAMA]: %v\n", gama)
+				}
+			case .pHYs:
+				if phys, phys_ok := phys(c); phys_ok {
 					if phys.unit == .Meter {
 						xm    := f32(img.width)  / f32(phys.ppu_x)
 						ym    := f32(img.height) / f32(phys.ppu_y)
@@ -80,73 +83,71 @@ demo :: proc() {
 					} else {
 						fmt.printf("[pHYs] x: %v, y: %v pixels per unknown unit.\n", phys.ppu_x, phys.ppu_y)
 					}
-				case .iTXt, .zTXt, .tEXt:
-					res, ok_text := text(c)
-					if ok_text {
-						if c.header.type == .iTXt {
-							fmt.printf("[iTXt] %v (%v:%v): %v\n", res.keyword, res.language, res.keyword_localized, res.text)
-						} else {
-							fmt.printf("[tEXt/zTXt] %v: %v\n", res.keyword, res.text)
-						}
-					}
-					defer text_destroy(res)
-				case .bKGD:
-					fmt.printf("[bKGD] %v\n", img.background)
-				case .eXIf:
-					res, ok_exif := exif(c)
-					if ok_exif {
-						/*
-							Other than checking the signature and byte order, we don't handle Exif data.
-							If you wish to interpret it, pass it to an Exif parser.
-						*/
-						fmt.printf("[eXIf] %v\n", res)
-					}
-				case .PLTE:
-					plte, plte_ok := plte(c)
-					if plte_ok {
-						fmt.printf("[PLTE] %v\n", plte)
-					} else {
-						fmt.printf("[PLTE] Error\n")
-					}
-				case .hIST:
-					res, ok_hist := hist(c)
-					if ok_hist {
-						fmt.printf("[hIST] %v\n", res)
-					}
-				case .cHRM:
-					res, ok_chrm := chrm(c)
-					if ok_chrm {
-						fmt.printf("[cHRM] %v\n", res)
-					}
-				case .sPLT:
-					res, ok_splt := splt(c)
-					if ok_splt {
-						fmt.printf("[sPLT] %v\n", res)
-					}
-					splt_destroy(res)
-				case .sBIT:
-					if res, ok_sbit := sbit(c); ok_sbit {
-						fmt.printf("[sBIT] %v\n", res)
-					}
-				case .iCCP:
-					res, ok_iccp := iccp(c)
-					if ok_iccp {
-						fmt.printf("[iCCP] %v\n", res)
-					}
-					iccp_destroy(res)
-				case .sRGB:
-					if res, ok_srgb := srgb(c); ok_srgb {
-						fmt.printf("[sRGB] Rendering intent: %v\n", res)
-					}
-				case:
-					type := c.header.type
-					name := chunk_type_to_name(&type)
-					fmt.printf("[%v]: %v\n", name, c.data)
 				}
+			case .iTXt, .zTXt, .tEXt:
+				res, ok_text := text(c)
+				if ok_text {
+					if c.header.type == .iTXt {
+						fmt.printf("[iTXt] %v (%v:%v): %v\n", res.keyword, res.language, res.keyword_localized, res.text)
+					} else {
+						fmt.printf("[tEXt/zTXt] %v: %v\n", res.keyword, res.text)
+					}
+				}
+				defer text_destroy(res)
+			case .bKGD:
+				fmt.printf("[bKGD] %v\n", img.background)
+			case .eXIf:
+				if res, ok_exif := exif(c); ok_exif {
+					/*
+						Other than checking the signature and byte order, we don't handle Exif data.
+						If you wish to interpret it, pass it to an Exif parser.
+					*/
+					fmt.printf("[eXIf] %v\n", res)
+				}
+			case .PLTE:
+				if plte, plte_ok := plte(c); plte_ok {
+					fmt.printf("[PLTE] %v\n", plte)
+				} else {
+					fmt.printf("[PLTE] Error\n")
+				}
+			case .hIST:
+				if res, ok_hist := hist(c); ok_hist {
+					fmt.printf("[hIST] %v\n", res)
+				}
+			case .cHRM:
+				if res, ok_chrm := chrm(c); ok_chrm {
+					fmt.printf("[cHRM] %v\n", res)
+				}
+			case .sPLT:
+				res, ok_splt := splt(c)
+				if ok_splt {
+					fmt.printf("[sPLT] %v\n", res)
+				}
+				splt_destroy(res)
+			case .sBIT:
+				if res, ok_sbit := sbit(c); ok_sbit {
+					fmt.printf("[sBIT] %v\n", res)
+				}
+			case .iCCP:
+				res, ok_iccp := iccp(c)
+				if ok_iccp {
+					fmt.printf("[iCCP] %v\n", res)
+				}
+				iccp_destroy(res)
+			case .sRGB:
+				if res, ok_srgb := srgb(c); ok_srgb {
+					fmt.printf("[sRGB] Rendering intent: %v\n", res)
+				}
+			case:
+				type := c.header.type
+				name := chunk_type_to_name(&type)
+				fmt.printf("[%v]: %v\n", name, c.data)
 			}
 		}
 	}
 
+	fmt.printf("Done parsing metadata.\n")
+
 	if err == nil && .do_not_decompress_image not_in options && .info not_in options {
 		if ok := write_image_as_ppm("out.ppm", img); ok {
 			fmt.println("Saved decoded image.")

core/image/png/helpers.odin

@@ -34,15 +34,17 @@ destroy :: proc(img: ^Image) {
 	}
 
 	bytes.buffer_destroy(&img.pixels)
+
+	assert(img.metadata_ptr != nil && img.metadata_type == Info)
+	v := (^Info)(img.metadata_ptr)
+
+	for chunk in &v.chunks {
+		delete(chunk.data)
+	}
+	delete(v.chunks)
+
 	// Clean up Info.
 	free(img.metadata_ptr)
-
-	/*
-		We don't need to do anything for the individual chunks.
-		They're allocated on the temp allocator, as is info.chunks
-
-		See read_chunk.
-	*/
 	free(img)
 }
 
@@ -50,46 +52,50 @@ destroy :: proc(img: ^Image) {
 	Chunk helpers
 */
 
-gamma :: proc(c: Chunk) -> f32 {
+gamma :: proc(c: Chunk) -> (res: f32, ok: bool) {
-	assert(c.header.type == .gAMA)
+	if c.header.type != .gAMA || len(c.data) != size_of(gAMA) {
-	res := (^gAMA)(raw_data(c.data))^
+		return {}, false
-	when true {
-		// Returns the wrong result on old backend
-		// Fixed for -llvm-api
-		return f32(res.gamma_100k) / 100_000.0
-	} else {
-		return f32(u32(res.gamma_100k)) / 100_000.0
 	}
+	gama := (^gAMA)(raw_data(c.data))^
+	return f32(gama.gamma_100k) / 100_000.0, true
 }
 
 INCHES_PER_METER :: 1000.0 / 25.4
 
-phys :: proc(c: Chunk) -> pHYs {
+phys :: proc(c: Chunk) -> (res: pHYs, ok: bool) {
-	assert(c.header.type == .pHYs)
+	if c.header.type != .pHYs || len(c.data) != size_of(pHYs) {
-	res := (^pHYs)(raw_data(c.data))^
+		return {}, false
-	return res
+	}
+
+	return (^pHYs)(raw_data(c.data))^, true 
 }
 
 phys_to_dpi :: proc(p: pHYs) -> (x_dpi, y_dpi: f32) {
 	return f32(p.ppu_x) / INCHES_PER_METER, f32(p.ppu_y) / INCHES_PER_METER
 }
 
-time :: proc(c: Chunk) -> tIME {
+time :: proc(c: Chunk) -> (res: tIME, ok: bool) {
-	assert(c.header.type == .tIME)
+	if c.header.type != .tIME || len(c.data) != size_of(tIME) {
-	res := (^tIME)(raw_data(c.data))^
+		return {}, false
-	return res
+	}
+
+	return (^tIME)(raw_data(c.data))^, true
 }
 
 core_time :: proc(c: Chunk) -> (t: coretime.Time, ok: bool) {
-	png_time := time(c)
+	if png_time, png_ok := time(c); png_ok {
-	using png_time
+		using png_time
-	return coretime.datetime_to_time(
+		return coretime.datetime_to_time(
-		int(year), int(month), int(day),
+			int(year), int(month), int(day),
-		int(hour), int(minute), int(second),
+			int(hour), int(minute), int(second),
-	)
+		)
+	} else {
+		return {}, false
+	}
 }
 
 text :: proc(c: Chunk) -> (res: Text, ok: bool) {
+	assert(len(c.data) == int(c.header.length))
 	#partial switch c.header.type {
 	case .tEXt:
 		ok = true
@@ -228,9 +234,7 @@ iccp_destroy :: proc(i: iCCP) {
 }
 
 srgb :: proc(c: Chunk) -> (res: sRGB, ok: bool) {
-	ok = true
+	if c.header.type != .sRGB || len(c.data) != size_of(sRGB_Rendering_Intent) {
-
-	if c.header.type != .sRGB || len(c.data) != 1 {
 		return {}, false
 	}
 
@@ -238,7 +242,7 @@ srgb :: proc(c: Chunk) -> (res: sRGB, ok: bool) {
 	if res.intent > max(sRGB_Rendering_Intent) {
 		ok = false; return
 	}
-	return
+	return res, true
 }
 
 plte :: proc(c: Chunk) -> (res: PLTE, ok: bool) {

core/image/png/png.odin

@@ -100,13 +100,13 @@ Chunk_Type :: enum u32be {
 }
 
 IHDR :: struct #packed {
-	width: u32be,
+	width:              u32be,
-	height: u32be,
+	height:             u32be,
-	bit_depth: u8,
+	bit_depth:          u8,
-	color_type: Color_Type,
+	color_type:         Color_Type,
 	compression_method: u8,
-	filter_method: u8,
+	filter_method:      u8,
-	interlace_method: Interlace_Method,
+	interlace_method:   Interlace_Method,
 }
 IHDR_SIZE :: size_of(IHDR)
 #assert (IHDR_SIZE == 13)
@@ -135,22 +135,22 @@ PLTE_Entry    :: [3]u8
 
 PLTE :: struct #packed {
 	entries: [256]PLTE_Entry,
-	used: u16,
+	used:    u16,
 }
 
 hIST :: struct #packed {
 	entries: [256]u16,
-	used: u16,
+	used:    u16,
 }
 
 sPLT :: struct #packed {
-	name: string,
+	name:    string,
-	depth: u8,
+	depth:   u8,
 	entries: union {
 		[][4]u8,
 		[][4]u16,
 	},
-	used: u16,
+	used:    u16,
 }
 
 // Other chunks
@@ -223,14 +223,14 @@ Exif :: struct {
 }
 
 iCCP :: struct {
-	name: string,
+	name:    string,
 	profile: []u8,
 }
 
 sRGB_Rendering_Intent :: enum u8 {
-	Perceptual = 0,
+	Perceptual            = 0,
 	Relative_colorimetric = 1,
-	Saturation = 2,
+	Saturation            = 2,
 	Absolute_colorimetric = 3,
 }
 
@@ -274,6 +274,35 @@ read_chunk :: proc(ctx: ^$C) -> (chunk: Chunk, err: Error) {
 	return chunk, nil
 }
 
+copy_chunk :: proc(src: Chunk, allocator := context.allocator) -> (dest: Chunk, err: Error) {
+	if int(src.header.length) != len(src.data) {
+		return {}, .Invalid_Chunk_Length
+	}
+
+	dest.header = src.header
+	dest.crc    = src.crc
+	dest.data   = make([]u8, dest.header.length, allocator) or_return
+
+	copy(dest.data[:], src.data[:])
+	return
+}
+
+append_chunk :: proc(list: ^[dynamic]Chunk, src: Chunk, allocator := context.allocator) -> (err: Error) {
+	if int(src.header.length) != len(src.data) {
+		return .Invalid_Chunk_Length
+	}
+
+	c := copy_chunk(src, allocator) or_return
+	length := len(list)
+	append(list, c)
+	if len(list) != length + 1 {
+		// Resize during append failed.
+		return .Out_Of_Memory
+	}
+
+	return
+}
+
 read_header :: proc(ctx: ^$C) -> (IHDR, Error) {
 	c, e := read_chunk(ctx)
 	if e != nil {
@@ -422,8 +451,6 @@ load_from_context :: proc(ctx: ^$C, options := Options{}, allocator := context.a
 
 	header:	IHDR
 
-	info.chunks.allocator = context.temp_allocator
-
 	// State to ensure correct chunk ordering.
 	seen_ihdr := false; first := true
 	seen_plte := false
@@ -518,7 +545,7 @@ load_from_context :: proc(ctx: ^$C, options := Options{}, allocator := context.a
 			}
 
 			if .return_metadata in options {
-				append(&info.chunks, c)
+				append_chunk(&info.chunks, c) or_return
 			}
 		case .IDAT:
 			// If we only want image metadata and don't want the pixel data, we can early out.
@@ -563,7 +590,7 @@ load_from_context :: proc(ctx: ^$C, options := Options{}, allocator := context.a
 			c = read_chunk(ctx) or_return
 			seen_bkgd = true
 			if .return_metadata in options {
-				append(&info.chunks, c)
+				append_chunk(&info.chunks, c) or_return
 			}
 
 			ct := transmute(u8)info.header.color_type
@@ -599,7 +626,7 @@ load_from_context :: proc(ctx: ^$C, options := Options{}, allocator := context.a
 			}
 
 			if .return_metadata in options {
-				append(&info.chunks, c)
+				append_chunk(&info.chunks, c) or_return
 			}
 
 			/*
@@ -626,16 +653,14 @@ load_from_context :: proc(ctx: ^$C, options := Options{}, allocator := context.a
 			/*
 				iPhone PNG bastardization that doesn't adhere to spec with broken IDAT chunk.
 				We're not going to add support for it. If you have the misfortunte of coming
-				across one of these files, use a utility to defry it.s
+				across one of these files, use a utility to defry it.
 			*/
 			return img, E_PNG.PNG_Does_Not_Adhere_to_Spec
 		case:
 			// Unhandled type
 			c = read_chunk(ctx) or_return
-
 			if .return_metadata in options {
-				// NOTE: Chunk cata is currently allocated on the temp allocator.
+				append_chunk(&info.chunks, c) or_return
-				append(&info.chunks, c)
 			}
 
 			first = false

tests/core/image/test_core_image.odin

@@ -1514,10 +1514,10 @@ run_png_suite :: proc(t: ^testing.T, suite: []PNG_Test) -> (subtotal: int) {
 							case .gAMA:
 								switch(file.file) {
 								case "pp0n2c16", "pp0n6a08":
-									gamma := png.gamma(c)
+									gamma, gamma_ok := png.gamma(c)
 									expected_gamma := f32(1.0)
 									error  = fmt.tprintf("%v test %v gAMA is %v, expected %v.", file.file, count, gamma, expected_gamma)
-									expect(t, gamma == expected_gamma, error)
+									expect(t, gamma == expected_gamma && gamma_ok, error)
 								}
 							case .PLTE:
 								switch(file.file) {
@@ -1557,25 +1557,25 @@ run_png_suite :: proc(t: ^testing.T, suite: []PNG_Test) -> (subtotal: int) {
 									expect(t, expected_chrm == chrm && chrm_ok, error)
 								}
 							case .pHYs:
-								phys     := png.phys(c)
+								phys, phys_ok := png.phys(c)
 								phys_err := "%v test %v cHRM is %v, expected %v."
 								switch (file.file) {
 								case "cdfn2c08":
 									expected_phys := png.pHYs{ppu_x =    1, ppu_y =    4, unit = .Unknown}
 									error  = fmt.tprintf(phys_err, file.file, count, phys, expected_phys)
-									expect(t, expected_phys == phys, error)
+									expect(t, expected_phys == phys && phys_ok, error)
 								case "cdhn2c08":
 									expected_phys := png.pHYs{ppu_x =    4, ppu_y =    1, unit = .Unknown}
 									error  = fmt.tprintf(phys_err, file.file, count, phys, expected_phys)
-									expect(t, expected_phys == phys, error)
+									expect(t, expected_phys == phys && phys_ok, error)
 								case "cdsn2c08":
 									expected_phys := png.pHYs{ppu_x =    1, ppu_y =    1, unit = .Unknown}
 									error  = fmt.tprintf(phys_err, file.file, count, phys, expected_phys)
-									expect(t, expected_phys == phys, error)
+									expect(t, expected_phys == phys && phys_ok, error)
 								case "cdun2c08":
 									expected_phys := png.pHYs{ppu_x = 1000, ppu_y = 1000, unit = .Meter}
 									error  = fmt.tprintf(phys_err, file.file, count, phys, expected_phys)
-									expect(t, expected_phys == phys, error)
+									expect(t, expected_phys == phys && phys_ok, error)
 								}
 							case .hIST:
 								hist, hist_ok := png.hist(c)
@@ -1589,7 +1589,7 @@ run_png_suite :: proc(t: ^testing.T, suite: []PNG_Test) -> (subtotal: int) {
 									expect(t, hist.used == 256 && hist_ok, error)
 								}
 							case .tIME:
-								png_time := png.time(c)
+								png_time, png_time_ok := png.time(c)
 								time_err := "%v test %v tIME was %v, expected %v."
 								expected_time: png.tIME
 
@@ -1610,7 +1610,7 @@ run_png_suite :: proc(t: ^testing.T, suite: []PNG_Test) -> (subtotal: int) {
 
 								}
 								error  = fmt.tprintf(time_err, file.file, count, png_time, expected_time)
-								expect(t, png_time == expected_time, error)
+								expect(t, png_time  == expected_time && png_time_ok,  error)
 
 								error  = fmt.tprintf(time_core_err, file.file, count, core_time, expected_core)
 								expect(t, core_time == expected_core && core_time_ok, error)