fftools/textformat: Apply quality improvements

Perform multiple improvements to increase code robustness. In particular: - favor unsigned counters for loops - add missing checks - avoid possible leaks - move variable declarations to inner scopes when feasible - provide explicit type-casting when needed

softworkz committed 1 year ago

Commit 5f90eea1 parent 8f42d907 files · +82 −45Browse files

Unified · Split

7 files

fftools/textformat/avtextformat.c +55 −27
fftools/textformat/avtextformat.h +2 −4
fftools/textformat/tf_default.c +6 −2
fftools/textformat/tf_ini.c +1 −1
fftools/textformat/tf_json.c +10 −7
fftools/textformat/tf_xml.c +0 −3
fftools/textformat/tw_avio.c +8 −1

fftools/textformat/avtextformat.c+55−27

@@ -93,9 +93,8 @@ static const AVClass textcontext_class = { · 92 unmodified lines
93	93
94	94	static void bprint_bytes(AVBPrint bp, const uint8_t ubuf, size_t ubuf_size)
95	95	{
96		− int i;
97	96	av_bprintf(bp, "0X");
98		− for (i = 0; i < ubuf_size; i++)
	97	+ for (unsigned i = 0; i < ubuf_size; i++)
99	98	av_bprintf(bp, "%02X", ubuf[i]);
100	99	}
101	100
@@ -137,6 +136,8 @@ int avtext_context_open(AVTextFormatContext *ptctx, const AVTextFormatter form · 35 unmodified lines
137	136	AVTextFormatContext *tctx;
138	137	int i, ret = 0;
139	138
	139	+ av_assert0(ptctx && formatter);
	140	+
140	141	if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
141	142	ret = AVERROR(ENOMEM);
142	143	goto fail;
@@ -215,8 +216,8 @@ int avtext_context_open(AVTextFormatContext *ptctx, const AVTextFormatter form · 72 unmodified lines
215	216
216	217	/* validate replace string */
217	218	{
218		− const uint8_t *p = tctx->string_validation_replacement;
219		− const uint8_t *endp = p + strlen(p);
	219	+ const uint8_t p = (uint8_t )tctx->string_validation_replacement;
	220	+ const uint8_t endp = p + strlen((const char )p);
220	221	while (*p) {
221	222	const uint8_t *p0 = p;
222	223	int32_t code;
@@ -229,6 +230,7 @@ int avtext_context_open(AVTextFormatContext *ptctx, const AVTextFormatter form · 6 unmodified lines
229	230	"Invalid UTF8 sequence %s found in string validation replace '%s'\n",
230	231	bp.str, tctx->string_validation_replacement);
231	232	return ret;
	233	+ goto fail;
232	234	}
233	235	}
234	236	}
@@ -256,6 +258,11 @@ static const char unit_bit_per_second_str[] = "bit/s"; · 21 unmodified lines
256	258
257	259	void avtext_print_section_header(AVTextFormatContext tctx, const void data, int section_id)
258	260	{
	261	+ if (section_id < 0 \|\| section_id >= tctx->nb_sections) {
	262	+ av_log(tctx, AV_LOG_ERROR, "Invalid section_id for section_header: %d\n", section_id);
	263	+ return;
	264	+ }
	265	+
259	266	tctx->level++;
260	267	av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
261	268
@@ -269,6 +276,11 @@ void avtext_print_section_header(AVTextFormatContext tctx, const void data, in · 7 unmodified lines
269	276
270	277	void avtext_print_section_footer(AVTextFormatContext *tctx)
271	278	{
	279	+ if (tctx->level < 0 \|\| tctx->level >= SECTION_MAX_NB_LEVELS) {
	280	+ av_log(tctx, AV_LOG_ERROR, "Invalid level for section_footer: %d\n", tctx->level);
	281	+ return;
	282	+ }
	283	+
272	284	int section_id = tctx->section[tctx->level]->id;
273	285	int parent_section_id = tctx->level ?
274	286	tctx->section[tctx->level - 1]->id : SECTION_ID_NONE;
@@ -285,7 +297,11 @@ void avtext_print_section_footer(AVTextFormatContext *tctx) · 10 unmodified lines
285	297
286	298	void avtext_print_integer(AVTextFormatContext tctx, const char key, int64_t val)
287	299	{
288		− const struct AVTextFormatSection *section = tctx->section[tctx->level];
	300	+ const AVTextFormatSection *section;
	301	+
	302	+ av_assert0(key && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS);
	303	+
	304	+ section = tctx->section[tctx->level];
289	305
290	306	if (section->show_all_entries \|\| av_dict_get(section->entries_to_show, key, NULL, 0)) {
291	307	tctx->formatter->print_integer(tctx, key, val);
@@ -354,17 +370,18 @@ struct unit_value { · 62 unmodified lines
354	370	const char *unit;
355	371	};
356	372
357		−static char value_string(AVTextFormatContext tctx, char *buf, int buf_size, struct unit_value uv)
	373	+static char value_string(const AVTextFormatContext tctx, char *buf, int buf_size, struct unit_value uv)
358	374	{
359	375	double vald;
360		− int64_t vali;
	376	+ int64_t vali = 0;
361	377	int show_float = 0;
362	378
363	379	if (uv.unit == unit_second_str) {
364	380	vald = uv.val.d;
365	381	show_float = 1;
366	382	} else {
367		− vald = vali = uv.val.i;
	383	+ vald = (double)uv.val.i;
	384	+ vali = uv.val.i;
368	385	}
369	386
370	387	if (uv.unit == unit_second_str && tctx->use_value_sexagesimal_format) {
@@ -383,17 +400,17 @@ static char value_string(AVTextFormatContext tctx, char *buf, int buf_size, st · 12 unmodified lines
383	400	int64_t index;
384	401
385	402	if (uv.unit == unit_byte_str && tctx->use_byte_value_binary_prefix) {
386		− index = (int64_t) (log2(vald)) / 10;
387		− index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
	403	+ index = (int64_t)(log2(vald) / 10);
	404	+ index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
388	405	vald /= si_prefixes[index].bin_val;
389	406	prefix_string = si_prefixes[index].bin_str;
390	407	} else {
391		− index = (int64_t) (log10(vald)) / 3;
392		− index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
	408	+ index = (int64_t)(log10(vald) / 3);
	409	+ index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
393	410	vald /= si_prefixes[index].dec_val;
394	411	prefix_string = si_prefixes[index].dec_str;
395	412	}
396		− vali = vald;
	413	+ vali = (int64_t)vald;
397	414	}
398	415
399	416	if (show_float \|\| (tctx->use_value_prefix && vald != (int64_t)vald))
@@ -421,9 +438,13 @@ void avtext_print_unit_int(AVTextFormatContext tctx, const char key, int value · 21 unmodified lines
421	438
422	439	int avtext_print_string(AVTextFormatContext tctx, const char key, const char *val, int flags)
423	440	{
424		− const struct AVTextFormatSection *section = tctx->section[tctx->level];
	441	+ const AVTextFormatSection *section;
425	442	int ret = 0;
426	443
	444	+ av_assert0(key && val && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS);
	445	+
	446	+ section = tctx->section[tctx->level];
	447	+
427	448	if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER \|\|
428	449	(tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
429	450	&& (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL)
@@ -465,12 +486,11 @@ void avtext_print_rational(AVTextFormatContext tctx, const char key, AVRationa · 35 unmodified lines
465	486	void avtext_print_time(AVTextFormatContext tctx, const char key,
466	487	int64_t ts, const AVRational *time_base, int is_duration)
467	488	{
468		− char buf[128];
469		−
470	489	if ((!is_duration && ts == AV_NOPTS_VALUE) \|\| (is_duration && ts == 0)) {
471	490	avtext_print_string(tctx, key, "N/A", AV_TEXTFORMAT_PRINT_STRING_OPTIONAL);
472	491	} else {
473		− double d = ts * av_q2d(*time_base);
	492	+ char buf[128];
	493	+ double d = av_q2d(time_base) ts;
474	494	struct unit_value uv;
475	495	uv.val.d = d;
476	496	uv.unit = unit_second_str;
@@ -491,7 +511,8 @@ void avtext_print_data(AVTextFormatContext tctx, const char name, · 14 unmodified lines
491	511	const uint8_t *data, int size)
492	512	{
493	513	AVBPrint bp;
494		− int offset = 0, l, i;
	514	+ unsigned offset = 0;
	515	+ int l, i;
495	516
496	517	av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
497	518	av_bprintf(&bp, "\n");
@@ -518,25 +539,29 @@ void avtext_print_data(AVTextFormatContext tctx, const char name, · 20 unmodified lines
518	539	void avtext_print_data_hash(AVTextFormatContext tctx, const char name,
519	540	const uint8_t *data, int size)
520	541	{
521		− char p, buf[AV_HASH_MAX_SIZE 2 + 64] = { 0 };
	542	+ char buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
	543	+ int len;
522	544
523	545	if (!tctx->hash)
524	546	return;
525	547
526	548	av_hash_init(tctx->hash);
527	549	av_hash_update(tctx->hash, data, size);
528		− snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
529		− p = buf + strlen(buf);
530		− av_hash_final_hex(tctx->hash, p, buf + sizeof(buf) - p);
	550	+ len = snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
	551	+ av_hash_final_hex(tctx->hash, (uint8_t *)&buf[len], (int)sizeof(buf) - len);
531	552	avtext_print_string(tctx, name, buf, 0);
532	553	}
533	554
534	555	void avtext_print_integers(AVTextFormatContext tctx, const char name,
535		− uint8_t data, int size, const char format,
536		− int columns, int bytes, int offset_add)
	556	+ uint8_t data, int size, const char format,
	557	+ int columns, int bytes, int offset_add)
537	558	{
538	559	AVBPrint bp;
539		− int offset = 0, l, i;
	560	+ unsigned offset = 0;
	561	+ int l, i;
	562	+
	563	+ if (!name \|\| !data \|\| !format \|\| columns <= 0 \|\| bytes <= 0)
	564	+ return;
540	565
541	566	av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
542	567	av_bprintf(&bp, "\n");
@@ -602,12 +627,15 @@ int avtextwriter_context_open(AVTextWriterContext *pwctx, const AVTextWriter w · 59 unmodified lines
602	627	AVTextWriterContext *wctx;
603	628	int ret = 0;
604	629
605		− if (!(wctx = av_mallocz(sizeof(AVTextWriterContext)))) {
	630	+ if (!pwctx \|\| !writer)
	631	+ return AVERROR(EINVAL);
	632	+
	633	+ if (!((wctx = av_mallocz(sizeof(AVTextWriterContext))))) {
606	634	ret = AVERROR(ENOMEM);
607	635	goto fail;
608	636	}
609	637
610		− if (!(wctx->priv = av_mallocz(writer->priv_size))) {
	638	+ if (writer->priv_size && !((wctx->priv = av_mallocz(writer->priv_size)))) {
611	639	ret = AVERROR(ENOMEM);
612	640	goto fail;
613	641	}

fftools/textformat/avtextformat.h+2−4

@@ -21,9 +21,7 @@ · 20 unmodified lines
21	21	#ifndef FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
22	22	#define FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
23	23
24		−#include <stddef.h>
25	24	#include <stdint.h>
26		−#include "libavutil/attributes.h"
27	25	#include "libavutil/dict.h"
28	26	#include "libavformat/avio.h"
29	27	#include "libavutil/bprint.h"
@@ -103,7 +101,7 @@ struct AVTextFormatContext { · 73 unmodified lines
103	101	unsigned int nb_item_type[SECTION_MAX_NB_LEVELS][SECTION_MAX_NB_SECTIONS];
104	102
105	103	/** section per each level */
106		− const struct AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
	104	+ const AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
107	105	AVBPrint section_pbuf[SECTION_MAX_NB_LEVELS]; ///< generic print buffer dedicated to each section,
108	106	/// used by various formatters
109	107
@@ -124,7 +122,7 @@ struct AVTextFormatContext { · 14 unmodified lines
124	122	#define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
125	123
126	124	int avtext_context_open(AVTextFormatContext *ptctx, const AVTextFormatter formatter, AVTextWriterContext writer_context, const char args,
127		− const struct AVTextFormatSection *sections, int nb_sections,
	125	+ const AVTextFormatSection *sections, int nb_sections,
128	126	int show_value_unit,
129	127	int use_value_prefix,
130	128	int use_byte_value_binary_prefix,

fftools/textformat/tf_default.c+6−2

@@ -68,9 +68,10 @@ DEFINE_FORMATTER_CLASS(default); · 67 unmodified lines
68	68	/* lame uppercasing routine, assumes the string is lower case ASCII */
69	69	static inline char upcase_string(char dst, size_t dst_size, const char *src)
70	70	{
71		− int i;
	71	+ unsigned i;
	72	+
72	73	for (i = 0; src[i] && i < dst_size - 1; i++)
73		− dst[i] = av_toupper(src[i]);
	74	+ dst[i] = (char)av_toupper(src[i]);
74	75	dst[i] = 0;
75	76	return dst;
76	77	}
@@ -106,6 +107,9 @@ static void default_print_section_footer(AVTextFormatContext *wctx) · 29 unmodified lines
106	107	const struct AVTextFormatSection *section = wctx->section[wctx->level];
107	108	char buf[32];
108	109
	110	+ if (!section)
	111	+ return;
	112	+
109	113	if (def->noprint_wrappers \|\| def->nested_section[wctx->level])
110	114	return;
111	115

fftools/textformat/tf_ini.c+1−1

@@ -91,7 +91,7 @@ static char ini_escape_str(AVBPrint dst, const char *src) · 90 unmodified lines
91	91	/* fallthrough */
92	92	default:
93	93	if ((unsigned char)c < 32)
94		− av_bprintf(dst, "\\x00%02x", c & 0xff);
	94	+ av_bprintf(dst, "\\x00%02x", (unsigned char)c);
95	95	else
96	96	av_bprint_chars(dst, c, 1);
97	97	break;

fftools/textformat/tf_json.c+10−7

@@ -80,13 +80,18 @@ static const char json_escape_str(AVBPrint dst, const char src, void log_ctx · 79 unmodified lines
80	80	static const char json_subst[] = { '"', '\\', 'b', 'f', 'n', 'r', 't', 0 };
81	81	const char *p;
82	82
	83	+ if (!src) {
	84	+ av_log(log_ctx, AV_LOG_WARNING, "Cannot escape NULL string, returning NULL\n");
	85	+ return NULL;
	86	+ }
	87	+
83	88	for (p = src; *p; p++) {
84	89	char s = strchr(json_escape, p);
85	90	if (s) {
86	91	av_bprint_chars(dst, '\\', 1);
87	92	av_bprint_chars(dst, json_subst[s - json_escape], 1);
88	93	} else if ((unsigned char)*p < 32) {
89		− av_bprintf(dst, "\\u00%02x", *p & 0xff);
	94	+ av_bprintf(dst, "\\u00%02x", (unsigned char)*p);
90	95	} else {
91	96	av_bprint_chars(dst, *p, 1);
92	97	}
@@ -100,11 +105,10 @@ static void json_print_section_header(AVTextFormatContext wctx, const void dat · 7 unmodified lines
100	105	{
101	106	JSONContext *json = wctx->priv;
102	107	AVBPrint buf;
103		− const struct AVTextFormatSection *section = wctx->section[wctx->level];
104		− const struct AVTextFormatSection *parent_section = wctx->level ?
105		− wctx->section[wctx->level-1] : NULL;
	108	+ const AVTextFormatSection *section = wctx->section[wctx->level];
	109	+ const AVTextFormatSection *parent_section = wctx->level ? wctx->section[wctx->level - 1] : NULL;
106	110
107		− if (wctx->level && wctx->nb_item[wctx->level-1])
	111	+ if (wctx->level && wctx->nb_item[wctx->level - 1])
108	112	writer_put_str(wctx, ",\n");
109	113
110	114	if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_WRAPPER) {
@@ -185,8 +189,7 @@ static void json_print_str(AVTextFormatContext wctx, const char key, const cha · 74 unmodified lines
185	189	static void json_print_int(AVTextFormatContext wctx, const char key, int64_t value)
186	190	{
187	191	JSONContext *json = wctx->priv;
188		− const struct AVTextFormatSection *parent_section = wctx->level ?
189		− wctx->section[wctx->level-1] : NULL;
	192	+ const AVTextFormatSection *parent_section = wctx->level ? wctx->section[wctx->level - 1] : NULL;
190	193	AVBPrint buf;
191	194
192	195	if (wctx->nb_item[wctx->level] \|\| (parent_section && parent_section->flags & AV_TEXTFORMAT_SECTION_FLAG_NUMBERING_BY_TYPE))

fftools/textformat/tf_xml.c+0−3

@@ -18,10 +18,7 @@ · 17 unmodified lines
18	18	* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
19	19	*/
20	20
21		−#include <limits.h>
22		−#include <stdarg.h>
23	21	#include <stdint.h>
24		−#include <stdio.h>
25	22	#include <string.h>
26	23
27	24	#include "avtextformat.h"

fftools/textformat/tw_avio.c+8−1

@@ -23,6 +23,7 @@ · 22 unmodified lines
23	23	#include <string.h>
24	24
25	25	#include "avtextwriters.h"
	26	+#include "libavutil/avassert.h"
26	27
27	28	#include "libavutil/error.h"
28	29
@@ -53,7 +54,7 @@ static void io_w8(AVTextWriterContext *wctx, int b) · 24 unmodified lines
53	54	static void io_put_str(AVTextWriterContext wctx, const char str)
54	55	{
55	56	IOWriterContext *ctx = wctx->priv;
56		− avio_write(ctx->avio_context, str, strlen(str));
	57	+ avio_write(ctx->avio_context, (const unsigned char *)str, (int)strlen(str));
57	58	}
58	59
59	60	static void io_printf(AVTextWriterContext wctx, const char fmt, ...)
@@ -81,6 +82,10 @@ int avtextwriter_create_file(AVTextWriterContext *pwctx, const char output_fil · 21 unmodified lines
81	82	IOWriterContext *ctx;
82	83	int ret;
83	84
	85	+ if (!output_filename \|\| !output_filename[0]) {
	86	+ av_log(NULL, AV_LOG_ERROR, "The output_filename cannot be NULL or empty\n");
	87	+ return AVERROR(EINVAL);
	88	+ }
84	89
85	90	ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
86	91	if (ret < 0)
@@ -106,6 +111,8 @@ int avtextwriter_create_avio(AVTextWriterContext *pwctx, AVIOContext avio_ctx, · 19 unmodified lines
106	111	IOWriterContext *ctx;
107	112	int ret;
108	113
	114	+ av_assert0(avio_ctx);
	115	+
109	116	ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
110	117	if (ret < 0)
111	118	return ret;

fftools/textformat: Apply quality improvements

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code