From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id 5E6DF4D9DA
for <ffmpegdev@gitmailbox.com>; Mon, 21 Apr 2025 17:17:06 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E8A33687DDD;
Mon, 21 Apr 2025 20:17:01 +0300 (EEST)
Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com
[209.85.218.49])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B3D3F687C17
for <ffmpeg-devel@ffmpeg.org>; Mon, 21 Apr 2025 20:16:55 +0300 (EEST)
Received: by mail-ej1-f49.google.com with SMTP id
a640c23a62f3a-ac25d2b2354so613202166b.1
for <ffmpeg-devel@ffmpeg.org>; Mon, 21 Apr 2025 10:16:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20230601; t=1745255815; x=1745860615; darn=ffmpeg.org;
h=user-agent:in-reply-to:content-disposition:mime-version:references
:mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc
:subject:date:message-id:reply-to;
bh=XbtOpsxZB68oVOXP2z77njy5mtJz2eE89s6W0wDFpHc=;
b=Hp6ph14Zbc4oK9ZALr2FsgTxvKgzyTlOf6HJonw6uZgRMBjGn+MUmAijh5hBtzToo4
G6aXJHQK3V4y/o0pdJU3DpCOeUPp0bswhObC3FI8ZoUT73ZlDu1gfcG8s8RobwTM66zR
FFbXAKmQnORUnmbCW+1m1O0uHeXHj48JIYkFSxZA9SHgsgEypaJZFfsxPmF6mgt0AYX5
x5BaI4lHy32Xwzhk+nP1Ya36qYyE16ltFVzkSbbJPSaUwUzVLIuhNwQDC1nXKLyV+io5
C0YkmBeJ6ywfr0jeP8ckFccIKtdv3IbjT1XFuTDMfFaE03m8nN8wzKjKW+STPwt8eSmg
FPTg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1745255815; x=1745860615;
h=user-agent:in-reply-to:content-disposition:mime-version:references
:mail-followup-to:message-id:subject:cc:to:from:date
:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
bh=XbtOpsxZB68oVOXP2z77njy5mtJz2eE89s6W0wDFpHc=;
b=dRXuAt/Wy4apqi8n0udHpK2b6ZcsAaiaw8QHMMF7MmCdsv4zsIBROQORJbAoJvIISc
kZRSf1yThDwqfRX5OTSdMKX45mr/Vy0RnMNE0sIPNShJLm7rBRpS8RLG2TXS0mjz9kwa
roOkIx3BeeguAt48CvzMRpYdexwAJTQIqYFBqtHNJOFIxikbUfXSrACwmEtCBFMOzL4u
wdIvUeLscDiHQ+bhlLCFwrBSxMyAvs5loxf5XpajNldGIw2TIab4qhPvcVCNnG8Cp7dP
Jxo6SwS0XQCIdKJ7PMkQpfCePG1Fhe3vOr7YJ/0HPczvYKOkA/yJyUW6Z7UZGcrclFFJ
YK+g==
X-Gm-Message-State: AOJu0Yx2lO8em/pfJ6ICbHn+Rtp7d3UBT4vsUgqZ9Ado1kdINIArn+G/
2udLDGSnepT7sYtAfn7JOhqAhCDe5o/7EB0NDigElsPnZKlK+3GaKmfp0g==
X-Gm-Gg: ASbGnctWAoB8i/lpI0HH/o+FpQdGV9S936WbyIVQJ9nqUSxVMVaSZtIPs5BOubA07Uj
P7B4z4c1G3cAnGSlK6HKRz3c6p1SWdxbQfe2MYRbJ9HWC0SWjzkXAaIDmNsDk12R++2FlAg1jLz
da+d5rkoHPAz1AHXl8+06+YfYmfIQT+64Am7D8wSgMKLjXxs37ffoiYGpinV7DvsyDcwQrmX6J6
Okiz8nUwuML73eXozsdUO8P2bhHjc0yp7J4QrIRSFQqKYG+pl0Io4uEdlIPAxGevkqozWZEYxjC
EL4SIDi0fH5sq8nXkzxUJ960hwb6PrAQnemJbByrwQC243xvATgYD3VrP78eQgU06QmA4jd/lRM
ljqwG
X-Google-Smtp-Source: AGHT+IEdW1zHXJXabufOPAlqRDlGhlZS0GlXosEW6E0yjAmQZccp5yOcmrMuxxZK7u/QmZy5z6kP7Q==
X-Received: by 2002:a17:907:7246:b0:ac6:f6ea:cc21 with SMTP id
a640c23a62f3a-acb74ddd317mr1311633966b.55.1745255814450;
Mon, 21 Apr 2025 10:16:54 -0700 (PDT)
Received: from mariano (dynamic-adsl-84-220-189-10.clienti.tiscali.it.
[84.220.189.10]) by smtp.gmail.com with ESMTPSA id
a640c23a62f3a-acb6ec0b1d3sm529852766b.4.2025.04.21.10.16.53
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Mon, 21 Apr 2025 10:16:53 -0700 (PDT)
Received: by mariano (Postfix, from userid 1000)
id 76D57BFCE8; Mon, 21 Apr 2025 19:16:52 +0200 (CEST)
Date: Mon, 21 Apr 2025 19:16:52 +0200
From: Stefano Sabatini <stefasab@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <aAZ9hFakRwILdeRP@mariano>
Mail-Followup-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>,
softworkz <softworkz-at-hotmail.com@ffmpeg.org>
References: <pull.66.v3.ffstaging.FFmpeg.1744945024.ffmpegagent@gmail.com>
<pull.66.v4.ffstaging.FFmpeg.1745189954.ffmpegagent@gmail.com>
<1516e67a5b216022a1ecc2292ee8e2989cfda192.1745189955.git.ffmpegagent@gmail.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <1516e67a5b216022a1ecc2292ee8e2989cfda192.1745189955.git.ffmpegagent@gmail.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
Subject: Re: [FFmpeg-devel] [PATCH v4 02/11] fftools/textformat: Quality
improvements
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: softworkz <softworkz@hotmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/aAZ9hFakRwILdeRP@mariano/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On date Sunday 2025-04-20 22:59:05 +0000, softworkz wrote:
> From: softworkz <softworkz@hotmail.com>
>
Nazigrammar commit log nit: use verb to specify the change action,
like in: apply quality improvements
Also probably want to have some more details below the headline,
like:
Perform multiple improvements to increase code robustness.
In particular:
* favor unsigned counters for loops
* add missing checks
* avoid possibly leaks
* move variable declarations to inner scopes when feasible
* provide explicit type-casting when needed
...
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> fftools/textformat/avtextformat.c | 111 +++++++++++++++++++-----------
> fftools/textformat/avtextformat.h | 6 +-
> fftools/textformat/tf_default.c | 8 ++-
> fftools/textformat/tf_ini.c | 2 +-
> fftools/textformat/tf_json.c | 8 ++-
> fftools/textformat/tf_xml.c | 3 -
> fftools/textformat/tw_avio.c | 9 ++-
> 7 files changed, 93 insertions(+), 54 deletions(-)
>
> diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
> index edbcd0b342..893b11298e 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]);
> }
>
> @@ -141,7 +140,10 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> AVTextFormatContext *tctx;
> int i, ret = 0;
>
> - if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> + if (!ptctx || !formatter)
> + return AVERROR(EINVAL);
> +
> + if (!((tctx = av_mallocz(sizeof(AVTextFormatContext))))) {
Sorry to nitpick and if I miss the past discussion, why the added
parentheses?
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> @@ -213,25 +215,26 @@ int avtext_context_open(AVTextFormatContext **ptctx,
> 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);
> + goto fail;
> }
> }
> }
> @@ -259,6 +262,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;
> +
> tctx->level++;
> av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
>
> @@ -272,6 +278,9 @@ void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, in
>
> void avtext_print_section_footer(AVTextFormatContext *tctx)
> {
> + if (tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> + return;
> +
> int section_id = tctx->section[tctx->level]->id;
> int parent_section_id = tctx->level
> ? tctx->section[tctx->level - 1]->id
> @@ -289,7 +298,12 @@ void avtext_print_section_footer(AVTextFormatContext *tctx)
>
> void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val)
> {
> - const struct AVTextFormatSection *section = tctx->section[tctx->level];
> + const AVTextFormatSection *section;
> +
> + if (!key || tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> + return;
possibly unrelated: maybe we should add an explicit warning or even an
assert?
> +
> + section = tctx->section[tctx->level];
>
> if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) {
> tctx->formatter->print_integer(tctx, key, val);
> @@ -299,24 +313,25 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
>
> static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const char *src)
> {
> - const uint8_t *p, *endp;
> + const uint8_t *p, *endp, *srcp = (const uint8_t *)src;
> AVBPrint dstbuf;
> + AVBPrint bp;
> int invalid_chars_nb = 0, ret = 0;
>
> + *dstp = NULL;
> av_bprint_init(&dstbuf, 0, AV_BPRINT_SIZE_UNLIMITED);
> + av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
>
> - endp = src + strlen(src);
> - for (p = src; *p;) {
> - uint32_t code;
> + endp = srcp + strlen(src);
> + for (p = srcp; *p;) {
> + int32_t code;
> int invalid = 0;
> const uint8_t *p0 = p;
>
> if (av_utf8_decode(&code, &p, endp, tctx->string_validation_utf8_flags) < 0) {
> - AVBPrint bp;
> - av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> - bprint_bytes(&bp, p0, p-p0);
> - av_log(tctx, AV_LOG_DEBUG,
> - "Invalid UTF-8 sequence %s found in string '%s'\n", bp.str, src);
> + av_bprint_clear(&bp);
> + bprint_bytes(&bp, p0, p - p0);
> + av_log(tctx, AV_LOG_DEBUG, "Invalid UTF-8 sequence %s found in string '%s'\n", bp.str, src);
> invalid = 1;
> }
>
> @@ -336,7 +351,7 @@ static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const
> }
>
> if (!invalid || tctx->string_validation == AV_TEXTFORMAT_STRING_VALIDATION_IGNORE)
> - av_bprint_append_data(&dstbuf, p0, p-p0);
> + av_bprint_append_data(&dstbuf, (const char *)p0, p - p0);
> }
>
> if (invalid_chars_nb && tctx->string_validation == AV_TEXTFORMAT_STRING_VALIDATION_REPLACE)
> @@ -346,6 +361,7 @@ static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const
>
> end:
> av_bprint_finalize(&dstbuf, dstp);
> + av_bprint_finalize(&bp, NULL);
> return ret;
> }
Please split this to a dedicated commit, we want to have a
justification for this one in the commit log.
>
> @@ -358,17 +374,18 @@ struct unit_value {
> const char *unit;
> };
>
> -static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv)
> +static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv)
> {
> double vald;
> - int64_t vali;
> + int64_t vali = 0;
> int show_float = 0;
>
> if (uv.unit == unit_second_str) {
> vald = uv.val.d;
> show_float = 1;
> } else {
> - vald = vali = uv.val.i;
> + vald = (double)uv.val.i;
> + vali = uv.val.i;
> }
>
> if (uv.unit == unit_second_str && tctx->use_value_sexagesimal_format) {
> @@ -387,17 +404,17 @@ static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, st
> int64_t index;
>
> if (uv.unit == unit_byte_str && tctx->use_byte_value_binary_prefix) {
> - index = (int64_t) (log2(vald)) / 10;
> - index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> + index = (int64_t)(log2(vald) / 10);
> + index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> vald /= si_prefixes[index].bin_val;
> prefix_string = si_prefixes[index].bin_str;
> } else {
> - index = (int64_t) (log10(vald)) / 3;
> - index = av_clip(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> + index = (int64_t)(log10(vald) / 3);
> + index = av_clip64(index, 0, FF_ARRAY_ELEMS(si_prefixes) - 1);
> vald /= si_prefixes[index].dec_val;
> prefix_string = si_prefixes[index].dec_str;
> }
> - vali = vald;
> + vali = (int64_t)vald;
> }
>
> if (show_float || (tctx->use_value_prefix && vald != (int64_t)vald))
> @@ -425,9 +442,14 @@ void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value
>
> int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags)
> {
> - const struct AVTextFormatSection *section = tctx->section[tctx->level];
> + const AVTextFormatSection *section;
> int ret = 0;
>
> + if (!key || !val || tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> + return AVERROR(EINVAL);
> +
> + section = tctx->section[tctx->level];
ditto
> +
> if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
> (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
> && (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL)
> @@ -469,12 +491,11 @@ void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRationa
> void avtext_print_time(AVTextFormatContext *tctx, const char *key,
> int64_t ts, const AVRational *time_base, int is_duration)
> {
> - char buf[128];
> -
> if ((!is_duration && ts == AV_NOPTS_VALUE) || (is_duration && ts == 0)) {
> avtext_print_string(tctx, key, "N/A", AV_TEXTFORMAT_PRINT_STRING_OPTIONAL);
> } else {
> - double d = ts * av_q2d(*time_base);
> + char buf[128];
> + double d = av_q2d(*time_base) * ts;
> struct unit_value uv;
> uv.val.d = d;
> uv.unit = unit_second_str;
> @@ -495,7 +516,8 @@ void avtext_print_data(AVTextFormatContext *tctx, const char *name,
> const uint8_t *data, int size)
> {
> AVBPrint bp;
> - int offset = 0, l, i;
> + unsigned offset = 0;
> + int l, i;
>
> av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> av_bprintf(&bp, "\n");
> @@ -522,25 +544,29 @@ void avtext_print_data(AVTextFormatContext *tctx, const char *name,
> void avtext_print_data_hash(AVTextFormatContext *tctx, const char *name,
> const uint8_t *data, int size)
> {
> - char *p, buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> + char buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> + int len;
>
> if (!tctx->hash)
> return;
>
> av_hash_init(tctx->hash);
> av_hash_update(tctx->hash, data, size);
> - snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
> - p = buf + strlen(buf);
> - av_hash_final_hex(tctx->hash, p, buf + sizeof(buf) - p);
> + len = snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash));
> + av_hash_final_hex(tctx->hash, (uint8_t *)&buf[len], (int)sizeof(buf) - len);
> avtext_print_string(tctx, name, buf, 0);
> }
>
> void avtext_print_integers(AVTextFormatContext *tctx, const char *name,
> - uint8_t *data, int size, const char *format,
> - int columns, int bytes, int offset_add)
> + uint8_t *data, int size, const char *format,
> + int columns, int bytes, int offset_add)
> {
> AVBPrint bp;
> - int offset = 0, l, i;
> + unsigned offset = 0;
> + int l, i;
> +
> + if (!name || !data || !format || columns <= 0 || bytes <= 0)
> + return;
>
> av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> av_bprintf(&bp, "\n");
> @@ -606,12 +632,15 @@ int avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter *w
> AVTextWriterContext *wctx;
> int ret = 0;
>
> - if (!(wctx = av_mallocz(sizeof(AVTextWriterContext)))) {
> + if (!pwctx || !writer)
> + return AVERROR(EINVAL);
> +
> + if (!((wctx = av_mallocz(sizeof(AVTextWriterContext))))) {
> ret = AVERROR(ENOMEM);
> goto fail;
> }
>
> - if (!(wctx->priv = av_mallocz(writer->priv_size))) {
> + if (writer->priv_size && !((wctx->priv = av_mallocz(writer->priv_size)))) {
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
> index c598af3450..aea691f351 100644
> --- a/fftools/textformat/avtextformat.h
> +++ b/fftools/textformat/avtextformat.h
> @@ -21,9 +21,7 @@
> #ifndef FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
> #define FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
>
> -#include <stddef.h>
> #include <stdint.h>
> -#include "libavutil/attributes.h"
> #include "libavutil/dict.h"
> #include "libavformat/avio.h"
> #include "libavutil/bprint.h"
> @@ -103,7 +101,7 @@ struct AVTextFormatContext {
> unsigned int nb_item_type[SECTION_MAX_NB_LEVELS][SECTION_MAX_NB_SECTIONS];
>
> /** section per each level */
> - const struct AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
> + const AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
> AVBPrint section_pbuf[SECTION_MAX_NB_LEVELS]; ///< generic print buffer dedicated to each section,
> /// used by various formatters
>
> @@ -124,7 +122,7 @@ struct AVTextFormatContext {
> #define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
>
> int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args,
> - const struct AVTextFormatSection *sections, int nb_sections,
> + const AVTextFormatSection *sections, int nb_sections,
> int show_value_unit,
> int use_value_prefix,
> int use_byte_value_binary_prefix,
> diff --git a/fftools/textformat/tf_default.c b/fftools/textformat/tf_default.c
> index 2c5047eafd..ad97173b0b 100644
> --- a/fftools/textformat/tf_default.c
> +++ b/fftools/textformat/tf_default.c
> @@ -68,9 +68,10 @@ DEFINE_FORMATTER_CLASS(default);
> /* lame uppercasing routine, assumes the string is lower case ASCII */
> static inline char *upcase_string(char *dst, size_t dst_size, const char *src)
> {
> - int i;
> + unsigned i;
> +
> for (i = 0; src[i] && i < dst_size - 1; i++)
> - dst[i] = av_toupper(src[i]);
> + dst[i] = (char)av_toupper(src[i]);
> dst[i] = 0;
> return dst;
> }
> @@ -106,6 +107,9 @@ static void default_print_section_footer(AVTextFormatContext *wctx)
> const struct AVTextFormatSection *section = wctx->section[wctx->level];
> char buf[32];
>
> + if (!section)
> + return;
> +
> if (def->noprint_wrappers || def->nested_section[wctx->level])
> return;
>
> diff --git a/fftools/textformat/tf_ini.c b/fftools/textformat/tf_ini.c
> index 88add0819a..dd77d0e8bf 100644
> --- a/fftools/textformat/tf_ini.c
> +++ b/fftools/textformat/tf_ini.c
> @@ -91,7 +91,7 @@ static char *ini_escape_str(AVBPrint *dst, const char *src)
> /* fallthrough */
> default:
> if ((unsigned char)c < 32)
> - av_bprintf(dst, "\\x00%02x", c & 0xff);
> + av_bprintf(dst, "\\x00%02x", (unsigned char)c);
> else
> av_bprint_chars(dst, c, 1);
> break;
> diff --git a/fftools/textformat/tf_json.c b/fftools/textformat/tf_json.c
> index b61d3740c6..e86cdbf5d9 100644
> --- a/fftools/textformat/tf_json.c
> +++ b/fftools/textformat/tf_json.c
> @@ -80,13 +80,18 @@ static const char *json_escape_str(AVBPrint *dst, const char *src, void *log_ctx
> static const char json_subst[] = { '"', '\\', 'b', 'f', 'n', 'r', 't', 0 };
> const char *p;
>
> + if (!src) {
> + av_log(log_ctx, AV_LOG_ERROR, "json_escape_str: NULL source string\n");
nit++: warning?
About error semantic policy, from a quick grep it looks like FFmpeg
codebase avoids references to the function name, and only provides a
simple error description, we might do with:
Cannot escape NULL string, returning NULL
> + return NULL;
> + }
> +
> for (p = src; *p; p++) {
> char *s = strchr(json_escape, *p);
> if (s) {
> av_bprint_chars(dst, '\\', 1);
> av_bprint_chars(dst, json_subst[s - json_escape], 1);
> } else if ((unsigned char)*p < 32) {
> - av_bprintf(dst, "\\u00%02x", *p & 0xff);
> + av_bprintf(dst, "\\u00%02x", (unsigned char)*p);
> } else {
> av_bprint_chars(dst, *p, 1);
> }
> @@ -105,6 +110,7 @@ static void json_print_section_header(AVTextFormatContext *wctx, const void *dat
> wctx->section[wctx->level-1] : NULL;
>
> if (wctx->level && wctx->nb_item[wctx->level-1])
> + if (wctx->level && wctx->nb_item[wctx->level - 1])
> writer_put_str(wctx, ",\n");
>
> if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_WRAPPER) {
> diff --git a/fftools/textformat/tf_xml.c b/fftools/textformat/tf_xml.c
> index befb39246d..28abfc6400 100644
> --- a/fftools/textformat/tf_xml.c
> +++ b/fftools/textformat/tf_xml.c
> @@ -18,10 +18,7 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> -#include <limits.h>
> -#include <stdarg.h>
> #include <stdint.h>
> -#include <stdio.h>
> #include <string.h>
>
> #include "avtextformat.h"
> diff --git a/fftools/textformat/tw_avio.c b/fftools/textformat/tw_avio.c
> index 6034f74ec9..d1b494b7b4 100644
> --- a/fftools/textformat/tw_avio.c
> +++ b/fftools/textformat/tw_avio.c
> @@ -53,7 +53,7 @@ static void io_w8(AVTextWriterContext *wctx, int b)
> static void io_put_str(AVTextWriterContext *wctx, const char *str)
> {
> IOWriterContext *ctx = wctx->priv;
> - avio_write(ctx->avio_context, str, strlen(str));
> + avio_write(ctx->avio_context, (const unsigned char *)str, (int)strlen(str));
> }
>
> static void io_printf(AVTextWriterContext *wctx, const char *fmt, ...)
> @@ -78,10 +78,12 @@ const AVTextWriter avtextwriter_avio = {
>
> int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_filename)
> {
> + if (!output_filename || !output_filename[0])
> + return AVERROR(EINVAL);
I'd add a warning to aid debugging.
> +
> IOWriterContext *ctx;
> int ret;
>
> -
> ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
> if (ret < 0)
> return ret;
> @@ -103,6 +105,9 @@ int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_fil
>
> int avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, int close_on_uninit)
> {
> + if (!pwctx || !avio_ctx)
> + return AVERROR(EINVAL);
> +
maybe warning in this case as well
[...]
Looks good to me otherwise.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".