Change the signature of sqfs_compressor_create to return an error code

Make sure the function has a way of telling the caller *why* it failed.

This way, the function can convey whether it had an internal error, an
allocation failure, whether the arguments are totaly nonsensical, or
simply that the compressor *or specific configuration* is not supported.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

9 files changed, 79 insertions, 50 deletions

diff --git a/lib/common/comp_lzo.c b/lib/common/comp_lzo.c
index 24f4009..0956e0e 100644
--- a/lib/common/comp_lzo.c
+++ b/lib/common/comp_lzo.c
@@ -217,25 +217,26 @@ static void lzo_destroy(sqfs_object_t *base)
 	free(base);
 }
 
-sqfs_compressor_t *lzo_compressor_create(const sqfs_compressor_config_t *cfg)
+int lzo_compressor_create(const sqfs_compressor_config_t *cfg,
+			  sqfs_compressor_t **out)
 {
 	sqfs_compressor_t *base;
 	lzo_compressor_t *lzo;
 	size_t scratch_size;
 
 	if (cfg->flags & ~SQFS_COMP_FLAG_GENERIC_ALL)
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 
 	if (cfg->opt.lzo.algorithm >= LZO_NUM_ALGS ||
 	    lzo_algs[cfg->opt.lzo.algorithm].compress == NULL) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	if (cfg->opt.lzo.algorithm == SQFS_LZO1X_999) {
 		if (cfg->opt.lzo.level > SQFS_LZO_MAX_LEVEL)
-			return NULL;
+			return SQFS_ERROR_UNSUPPORTED;
 	} else if (cfg->opt.lzo.level != 0) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	/* XXX: liblzo does not do bounds checking internally,
@@ -257,7 +258,7 @@ sqfs_compressor_t *lzo_compressor_create(const sqfs_compressor_config_t *cfg)
 	base = (sqfs_compressor_t *)lzo;
 
 	if (lzo == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	lzo->block_size = cfg->block_size;
 	lzo->algorithm = cfg->opt.lzo.algorithm;
@@ -272,5 +273,7 @@ sqfs_compressor_t *lzo_compressor_create(const sqfs_compressor_config_t *cfg)
 	base->read_options = lzo_read_options;
 	((sqfs_object_t *)base)->copy = lzo_create_copy;
 	((sqfs_object_t *)base)->destroy = lzo_destroy;
-	return base;
+
+	*out = base;
+	return 0;
 }
diff --git a/lib/common/writer.c b/lib/common/writer.c
index 4762b82..84a98a5 100644
--- a/lib/common/writer.c
+++ b/lib/common/writer.c
@@ -85,19 +85,19 @@ int sqfs_writer_init(sqfs_writer_t *sqfs, const sqfs_writer_cfg_t *wrcfg)
 	if (fstree_init(&sqfs->fs, wrcfg->fs_defaults))
 		goto fail_file;
 
-	sqfs->cmp = sqfs_compressor_create(&cfg);
+	ret = sqfs_compressor_create(&cfg, &sqfs->cmp);
 
 #ifdef WITH_LZO
 	if (cfg.id == SQFS_COMP_LZO) {
 		if (sqfs->cmp != NULL)
 			sqfs_destroy(sqfs->cmp);
 
-		sqfs->cmp = lzo_compressor_create(&cfg);
+		ret = lzo_compressor_create(&cfg, &sqfs->cmp);
 	}
 #endif
 
-	if (sqfs->cmp == NULL) {
-		fputs("Error creating compressor\n", stderr);
+	if (ret != 0) {
+		sqfs_perror(wrcfg->filename, "creating compressor", ret);
 		goto fail_fs;
 	}
 
diff --git a/lib/sqfs/comp/compressor.c b/lib/sqfs/comp/compressor.c
index 28de2c5..3b48030 100644
--- a/lib/sqfs/comp/compressor.c
+++ b/lib/sqfs/comp/compressor.c
@@ -12,8 +12,8 @@
 
 #include "internal.h"
 
-typedef sqfs_compressor_t *(*compressor_fun_t)
-	(const sqfs_compressor_config_t *cfg);
+typedef int (*compressor_fun_t)(const sqfs_compressor_config_t *cfg,
+				sqfs_compressor_t **out);
 
 static compressor_fun_t compressors[SQFS_COMP_MAX + 1] = {
 #ifdef WITH_GZIP
@@ -81,17 +81,24 @@ bool sqfs_compressor_exists(SQFS_COMPRESSOR id)
 	return (compressors[id] != NULL);
 }
 
-sqfs_compressor_t *sqfs_compressor_create(const sqfs_compressor_config_t *cfg)
+int sqfs_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out)
 {
 	sqfs_u8 padd0[sizeof(cfg->opt)];
 	int ret;
 
-	if (cfg == NULL || cfg->id < SQFS_COMP_MIN || cfg->id > SQFS_COMP_MAX)
-		return NULL;
+	/* check compressor ID */
+	if (cfg == NULL)
+		return SQFS_ERROR_ARG_INVALID;
+
+	if (cfg->id < SQFS_COMP_MIN || cfg->id > SQFS_COMP_MAX)
+		return SQFS_ERROR_UNSUPPORTED;
 
 	if (compressors[cfg->id] == NULL)
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 
+	/* make sure the padding bytes are cleared, so we could theoretically
+	   turn them into option fields in the future and remain compatible */
 	memset(padd0, 0, sizeof(padd0));
 
 	switch (cfg->id) {
@@ -117,9 +124,9 @@ sqfs_compressor_t *sqfs_compressor_create(const sqfs_compressor_config_t *cfg)
 	}
 
 	if (ret != 0)
-		return NULL;
+		return SQFS_ERROR_ARG_INVALID;
 
-	return compressors[cfg->id](cfg);
+	return compressors[cfg->id](cfg, out);
 }
 
 const char *sqfs_compressor_name_from_id(SQFS_COMPRESSOR id)
diff --git a/lib/sqfs/comp/gzip.c b/lib/sqfs/comp/gzip.c
index 351d733..d702da1 100644
--- a/lib/sqfs/comp/gzip.c
+++ b/lib/sqfs/comp/gzip.c
@@ -247,7 +247,8 @@ static sqfs_object_t *gzip_create_copy(const sqfs_object_t *cmp)
 	return (sqfs_object_t *)gzip;
 }
 
-sqfs_compressor_t *gzip_compressor_create(const sqfs_compressor_config_t *cfg)
+int gzip_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out)
 {
 	gzip_compressor_t *gzip;
 	sqfs_compressor_t *base;
@@ -255,24 +256,24 @@ sqfs_compressor_t *gzip_compressor_create(const sqfs_compressor_config_t *cfg)
 
 	if (cfg->flags & ~(SQFS_COMP_FLAG_GZIP_ALL |
 			   SQFS_COMP_FLAG_GENERIC_ALL)) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	if (cfg->opt.gzip.level < SQFS_GZIP_MIN_LEVEL ||
 	    cfg->opt.gzip.level > SQFS_GZIP_MAX_LEVEL) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	if (cfg->opt.gzip.window_size < SQFS_GZIP_MIN_WINDOW ||
 	    cfg->opt.gzip.window_size > SQFS_GZIP_MAX_WINDOW) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	gzip = calloc(1, sizeof(*gzip));
 	base = (sqfs_compressor_t *)gzip;
 
 	if (gzip == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	gzip->opt.level = cfg->opt.gzip.level;
 	gzip->opt.window = cfg->opt.gzip.window_size;
@@ -296,8 +297,9 @@ sqfs_compressor_t *gzip_compressor_create(const sqfs_compressor_config_t *cfg)
 
 	if (ret != Z_OK) {
 		free(gzip);
-		return NULL;
+		return SQFS_ERROR_COMPRESSOR;
 	}
 
-	return base;
+	*out = base;
+	return 0;
 }
diff --git a/lib/sqfs/comp/internal.h b/lib/sqfs/comp/internal.h
index 7518f56..0566683 100644
--- a/lib/sqfs/comp/internal.h
+++ b/lib/sqfs/comp/internal.h
@@ -24,18 +24,23 @@ SQFS_INTERNAL
 int sqfs_generic_read_options(sqfs_file_t *file, void *data, size_t size);
 
 SQFS_INTERNAL
-sqfs_compressor_t *xz_compressor_create(const sqfs_compressor_config_t *cfg);
+int xz_compressor_create(const sqfs_compressor_config_t *cfg,
+			 sqfs_compressor_t **out);
 
 SQFS_INTERNAL
-sqfs_compressor_t *gzip_compressor_create(const sqfs_compressor_config_t *cfg);
+int gzip_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out);
 
 SQFS_INTERNAL
-sqfs_compressor_t *lz4_compressor_create(const sqfs_compressor_config_t *cfg);
+int lz4_compressor_create(const sqfs_compressor_config_t *cfg,
+			  sqfs_compressor_t **out);
 
 SQFS_INTERNAL
-sqfs_compressor_t *zstd_compressor_create(const sqfs_compressor_config_t *cfg);
+int zstd_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out);
 
 SQFS_INTERNAL
-sqfs_compressor_t *lzma_compressor_create(const sqfs_compressor_config_t *cfg);
+int lzma_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out);
 
 #endif /* INTERNAL_H */
diff --git a/lib/sqfs/comp/lz4.c b/lib/sqfs/comp/lz4.c
index 460ac44..4791746 100644
--- a/lib/sqfs/comp/lz4.c
+++ b/lib/sqfs/comp/lz4.c
@@ -132,20 +132,21 @@ static void lz4_destroy(sqfs_object_t *base)
 	free(base);
 }
 
-sqfs_compressor_t *lz4_compressor_create(const sqfs_compressor_config_t *cfg)
+int lz4_compressor_create(const sqfs_compressor_config_t *cfg,
+			  sqfs_compressor_t **out)
 {
 	sqfs_compressor_t *base;
 	lz4_compressor_t *lz4;
 
 	if (cfg->flags & ~(SQFS_COMP_FLAG_LZ4_ALL |
 			   SQFS_COMP_FLAG_GENERIC_ALL)) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	lz4 = calloc(1, sizeof(*lz4));
 	base = (sqfs_compressor_t *)lz4;
 	if (lz4 == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	lz4->high_compression = (cfg->flags & SQFS_COMP_FLAG_LZ4_HC) != 0;
 	lz4->block_size = cfg->block_size;
@@ -157,5 +158,7 @@ sqfs_compressor_t *lz4_compressor_create(const sqfs_compressor_config_t *cfg)
 	base->read_options = lz4_read_options;
 	((sqfs_object_t *)base)->copy = lz4_create_copy;
 	((sqfs_object_t *)base)->destroy = lz4_destroy;
-	return base;
+
+	*out = base;
+	return 0;
 }
diff --git a/lib/sqfs/comp/lzma.c b/lib/sqfs/comp/lzma.c
index 51afe65..2a6cd1f 100644
--- a/lib/sqfs/comp/lzma.c
+++ b/lib/sqfs/comp/lzma.c
@@ -171,18 +171,19 @@ static void lzma_destroy(sqfs_object_t *base)
 	free(base);
 }
 
-sqfs_compressor_t *lzma_compressor_create(const sqfs_compressor_config_t *cfg)
+int lzma_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out)
 {
 	sqfs_compressor_t *base;
 	lzma_compressor_t *lzma;
 
 	if (cfg->flags & ~SQFS_COMP_FLAG_GENERIC_ALL)
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 
 	lzma = calloc(1, sizeof(*lzma));
 	base = (sqfs_compressor_t *)lzma;
 	if (lzma == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	lzma->block_size = cfg->block_size;
 
@@ -196,5 +197,7 @@ sqfs_compressor_t *lzma_compressor_create(const sqfs_compressor_config_t *cfg)
 	base->read_options = lzma_read_options;
 	((sqfs_object_t *)base)->copy = lzma_create_copy;
 	((sqfs_object_t *)base)->destroy = lzma_destroy;
-	return base;
+
+	*out = base;
+	return 0;
 }
diff --git a/lib/sqfs/comp/xz.c b/lib/sqfs/comp/xz.c
index 423604b..32fb032 100644
--- a/lib/sqfs/comp/xz.c
+++ b/lib/sqfs/comp/xz.c
@@ -227,23 +227,24 @@ static void xz_destroy(sqfs_object_t *base)
 	free(base);
 }
 
-sqfs_compressor_t *xz_compressor_create(const sqfs_compressor_config_t *cfg)
+int xz_compressor_create(const sqfs_compressor_config_t *cfg,
+			 sqfs_compressor_t **out)
 {
 	sqfs_compressor_t *base;
 	xz_compressor_t *xz;
 
 	if (cfg->flags & ~(SQFS_COMP_FLAG_GENERIC_ALL |
 			   SQFS_COMP_FLAG_XZ_ALL)) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	if (!is_dict_size_valid(cfg->opt.xz.dict_size))
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 
 	xz = calloc(1, sizeof(*xz));
 	base = (sqfs_compressor_t *)xz;
 	if (xz == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	xz->flags = cfg->flags;
 	xz->dict_size = cfg->opt.xz.dict_size;
@@ -255,5 +256,7 @@ sqfs_compressor_t *xz_compressor_create(const sqfs_compressor_config_t *cfg)
 	base->read_options = xz_read_options;
 	((sqfs_object_t *)base)->copy = xz_create_copy;
 	((sqfs_object_t *)base)->destroy = xz_destroy;
-	return base;
+
+	*out = base;
+	return 0;
 }
diff --git a/lib/sqfs/comp/zstd.c b/lib/sqfs/comp/zstd.c
index 12ddfdb..986c669 100644
--- a/lib/sqfs/comp/zstd.c
+++ b/lib/sqfs/comp/zstd.c
@@ -134,29 +134,30 @@ static void zstd_destroy(sqfs_object_t *base)
 	free(zstd);
 }
 
-sqfs_compressor_t *zstd_compressor_create(const sqfs_compressor_config_t *cfg)
+int zstd_compressor_create(const sqfs_compressor_config_t *cfg,
+			   sqfs_compressor_t **out)
 {
 	zstd_compressor_t *zstd;
 	sqfs_compressor_t *base;
 
 	if (cfg->flags & ~SQFS_COMP_FLAG_GENERIC_ALL)
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 
 	if (cfg->opt.zstd.level < 1 ||
 	    cfg->opt.zstd.level > ZSTD_maxCLevel()) {
-		return NULL;
+		return SQFS_ERROR_UNSUPPORTED;
 	}
 
 	zstd = calloc(1, sizeof(*zstd));
 	base = (sqfs_compressor_t *)zstd;
 	if (zstd == NULL)
-		return NULL;
+		return SQFS_ERROR_ALLOC;
 
 	zstd->block_size = cfg->block_size;
 	zstd->zctx = ZSTD_createCCtx();
 	if (zstd->zctx == NULL) {
 		free(zstd);
-		return NULL;
+		return SQFS_ERROR_COMPRESSOR;
 	}
 
 	base->get_configuration = zstd_get_configuration;
@@ -166,5 +167,7 @@ sqfs_compressor_t *zstd_compressor_create(const sqfs_compressor_config_t *cfg)
 	base->read_options = zstd_read_options;
 	((sqfs_object_t *)base)->copy = zstd_create_copy;
 	((sqfs_object_t *)base)->destroy = zstd_destroy;
-	return base;
+
+	*out = base;
+	return 0;
 }