On date Friday 2025-04-25 23:30:57 +0000, softworkz wrote:
> From: softworkz <softwo...@hotmail.com>
>
> 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
>
> Signed-off-by: softworkz <softwo...@hotmail.com>
> ---
> fftools/textformat/avtextformat.c | 85 ++++++++++++++++++++-----------
> fftools/textformat/avtextformat.h | 6 +--
> fftools/textformat/tf_default.c | 8 ++-
> fftools/textformat/tf_ini.c | 2 +-
> fftools/textformat/tf_json.c | 17 ++++---
> fftools/textformat/tf_xml.c | 3 --
> fftools/textformat/tw_avio.c | 11 +++-
> 7 files changed, 83 insertions(+), 49 deletions(-)
>
> diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> index 74d179c516..1939a1f739 100644
> --- a/fftools/textformat/avtextformat.c
> +++ b/fftools/textformat/avtextformat.c
> @@ -93,9 +93,8 @@ static const AVClass textcontext_class = {
>
> static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t ubuf_size)
> {
> - int i;
> av_bprintf(bp, "0X");
> - for (i = 0; i < ubuf_size; i++)
> + for (unsigned i = 0; i < ubuf_size; i++)
> av_bprintf(bp, "%02X", ubuf[i]);
> }
>
> @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> const AVTextFormatter *form
> AVTextFormatContext *tctx;
> int i, ret = 0;
>
> + if (!ptctx || !formatter)
> + return AVERROR(EINVAL);
assert0?
> +
> if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> ret = AVERROR(ENOMEM);
> goto fail;
> @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> const AVTextFormatter *form
> av_log(NULL, AV_LOG_ERROR, " %s", n);
> av_log(NULL, AV_LOG_ERROR, "\n");
> }
> - return ret;
> + goto fail;
> }
>
> /* validate replace string */
> {
> - const uint8_t *p = tctx->string_validation_replacement;
> - const uint8_t *endp = p + strlen(p);
> + const uint8_t *p = (uint8_t *)tctx->string_validation_replacement;
> + const uint8_t *endp = p + strlen((const char *)p);
> while (*p) {
> const uint8_t *p0 = p;
> int32_t code;
> ret = av_utf8_decode(&code, &p, endp,
> tctx->string_validation_utf8_flags);
> if (ret < 0) {
> AVBPrint bp;
> - av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> + av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> bprint_bytes(&bp, p0, p - p0),
> av_log(tctx, AV_LOG_ERROR,
> "Invalid UTF8 sequence %s found in string
> validation replace '%s'\n",
> bp.str, tctx->string_validation_replacement);
> - return ret;
> + av_bprint_finalize(&bp, NULL);
I see no advantage in this, since it's adding dynamic allocation and
the need to free which was previously not needed
> + goto fail;
> }
> }
> }
> @@ -255,6 +258,9 @@ static const char unit_bit_per_second_str[] = "bit/s";
>
> void avtext_print_section_header(AVTextFormatContext *tctx, const void
> *data, int section_id)
> {
> + if (section_id < 0 || section_id >= tctx->nb_sections)
> + return;
Given this is a check on the input data, we should probably make it
fail with a log message, meaning we should also check the return value
from the callee.
Given this is not a public API I'm not against the assert though.
[...]
> int avtextwriter_create_file(AVTextWriterContext **pwctx, const char
> *output_filename)
> {
> + if (!output_filename || !output_filename[0]) {
> + av_log(NULL, AV_LOG_ERROR, "The output_filename cannot be NULL or
> empty\n");
> + return AVERROR(EINVAL);
> + }
> +
> IOWriterContext *ctx;
> int ret;
nit here and below: move declarations before actual code for stylistic
consistency
[...]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
