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 78C634DC27
for <ffmpegdev@gitmailbox.com>; Wed, 23 Apr 2025 22:34:49 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 43A8768B207;
Thu, 24 Apr 2025 01:34:46 +0300 (EEST)
Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com
[209.85.208.41])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4F6AE687DAF
for <ffmpeg-devel@ffmpeg.org>; Thu, 24 Apr 2025 01:34:40 +0300 (EEST)
Received: by mail-ed1-f41.google.com with SMTP id
4fb4d7f45d1cf-5e61d91a087so495058a12.0
for <ffmpeg-devel@ffmpeg.org>; Wed, 23 Apr 2025 15:34:40 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20230601; t=1745447679; x=1746052479; 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=iKQ+s6iXrGgTKfFYR2pTgPoPHj1VsWeIPcR+5AEQHoI=;
b=Bg6ARsyqVmkYxgOlRTrG4bmFlR11FB0MHm35mU7CqqFMXN23k6TUZ+iTRAcad+nTQV
Vqj1dt2cxtey7VGBk8KxpgfEypNHElBJBJm1fZgvdDBXFlQeY0ZrsGEbevUpmfQpQONx
Cdu88jiLR3PIgQcLBLJsALjZzukmLqsyqYSY1t8IAATQk+1g7qG0oT9StxTNlqiiuSHb
YabU/XiiUEKUFYOQpTSGCRby6TgQRAC0yxBNBiHv5NpLBXBOBbOWLJrCSgNRrWsQEW09
d8YoaiLdri+Au7P0pSQ0kDgxbqDdMgB7IzKmphmMLYbqt3tDCZa7cVMlrfhh6bvt5rma
bB3A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1745447679; x=1746052479;
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=iKQ+s6iXrGgTKfFYR2pTgPoPHj1VsWeIPcR+5AEQHoI=;
b=ZjmeJFui1U8hyxpXrIqInm/8hcVF8273Akr7BsKdGBfwITb9ywrKpwmEwZ71HacVzS
ZRBCJMkPJFpsq5K3EOKuFw506n7P6FmAirHYXbLQOtFU479oAD3ICA+ePErsR9zoPMuT
KXJT87pQgZQPHKbSqsyEq7nmig8vNwhTSv3xqYw+gXI6tznhkhqxXsMcKEfZnODUIUn2
mxbjcGnDYyYprSSR3I1FJmmlDW2FkzSBoL6+TOf+g1eb2P4qPtBexfGrLoyFsIksBcLn
Jnv78pjoy20gfCgaY2/G0KvXNOHmE17pDdeW/JAjLUtlRn/4cmiwHRzws60bp/ZlpjNp
1WmQ==
X-Gm-Message-State: AOJu0YxkDjbcESJul3KV+xy0O6OTmYV9Hj1dhrRje8wmL9knNcuQ91uD
62/k47UDL3bo/KmkfIavvKKl3ynn1iLeKFXyBs7J/zyFG6dlaoSSttrQaA==
X-Gm-Gg: ASbGncv6BwE+R6UIiBQsEDEFOsd4GMxud/mfZ31QolOtOZR+UhYN8GiQGyRIKK3iK0p
WYXqWV1i6fqgrG+GpcOBj18uN9KFZldPUbRfcPA/HJCJ0qem8bibLAMmjmSI3pv2Fu0vxF2eF3M
ijw7iCPfztE3Ueyl96FfdteQqVBciOwxefZgMpwHmHtmilALbTR6y9LhaoQaOM8qDe7FabqyC6k
dZgXwc2iaGZmVCIGA3UNWHPXohGJwU0JSanuVWqXLVIDstYd6k3UI5J1042dJUdKsEtolvFkbyv
sF9rbs5evTx9P1ClHYgh/yX/anJxzs9IPe0JK74BbbzRjUuuWREPvBJZU6uMnsItTItrNVuJdk/
BY8Y6DDigWcmroXE=
X-Google-Smtp-Source: AGHT+IGoO+15A/bPuIOWinVQqy5B15+5RfTZ1WJWENaJLEwAkT9MlXZVvQ8XnPNlAEb7XmMgy5EPzQ==
X-Received: by 2002:a05:6402:274d:b0:5f4:d4e7:3c37 with SMTP id
4fb4d7f45d1cf-5f6de1c0d4dmr556313a12.6.1745447679087;
Wed, 23 Apr 2025 15:34:39 -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
4fb4d7f45d1cf-5f6eb3236d9sm198992a12.1.2025.04.23.15.34.37
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Wed, 23 Apr 2025 15:34:37 -0700 (PDT)
Received: by mariano (Postfix, from userid 1000)
id 2850BBFCE8; Thu, 24 Apr 2025 00:34:36 +0200 (CEST)
Date: Thu, 24 Apr 2025 00:34:36 +0200
From: Stefano Sabatini <stefasab@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <aAlq/OjEAcIYsm1E@mariano>
Mail-Followup-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>,
softworkz <softworkz-at-hotmail.com@ffmpeg.org>
References: <pull.66.v4.ffstaging.FFmpeg.1745189954.ffmpegagent@gmail.com>
<pull.66.v5.ffstaging.FFmpeg.1745358943.ffmpegagent@gmail.com>
<1e312f4685e66a4d266beabd39b8a8530a090bda.1745358943.git.ffmpegagent@gmail.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <1e312f4685e66a4d266beabd39b8a8530a090bda.1745358943.git.ffmpegagent@gmail.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
Subject: Re: [FFmpeg-devel] [PATCH v5 02/14] fftools/textformat: Apply
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/aAlq%2FOjEAcIYsm1E@mariano/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On date Tuesday 2025-04-22 21:55:31 +0000, softworkz wrote:
> From: softworkz <softworkz@hotmail.com>
>
> Perform multiple improvements to increase code robustness.
> In particular:
> - favor unsigned counters for loops
> - add missing checks
> - avoid possibly leaks
my typo: possible leaks
> - move variable declarations to inner scopes when feasible
> - provide explicit type-casting when needed
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
General nit about headline caseing: from the log most commit use
all lowercase in headline (I personally only use that form and at some
point everybody was using that).
> 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);
> +
> 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),
Is this really needed? AUTOMATIC should be faster since it will avoid
dynamic allocation and the need to finalize (although there is no
practical need for such optimization, this still seems the simplest
possible path). Besides, an UTF8 sequence cannot be longer than a few
bytes, so possibly av_utf8_decode cannot decode more than a few bytes.
> 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;
> }
> }
> }
> @@ -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;
> +
> tctx->level++;
> av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
>
> @@ -268,6 +274,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
> @@ -285,7 +294,11 @@ 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;
> +
> + av_assert0(key && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS);
> +
> + 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);
> @@ -354,17 +367,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) {
> @@ -383,17 +397,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))
> @@ -421,9 +435,13 @@ 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;
>
> + av_assert0(key && val && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS);
> +
> + section = tctx->section[tctx->level];
> +
> if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
> (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
> && (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL)
> @@ -465,12 +483,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;
> @@ -491,7 +508,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");
> @@ -518,25 +536,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");
> @@ -602,12 +624,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..50c3d90440 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_WARNING, "Cannot escape NULL string, returning NULL\n");
> + 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);
> }
> @@ -100,11 +105,10 @@ static void json_print_section_header(AVTextFormatContext *wctx, const void *dat
> {
> JSONContext *json = wctx->priv;
> AVBPrint buf;
> - const struct AVTextFormatSection *section = wctx->section[wctx->level];
> - const struct AVTextFormatSection *parent_section = wctx->level ?
> - wctx->section[wctx->level-1] : NULL;
> + const AVTextFormatSection *section = wctx->section[wctx->level];
> + const AVTextFormatSection *parent_section = wctx->level ? 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) {
> @@ -185,8 +189,7 @@ static void json_print_str(AVTextFormatContext *wctx, const char *key, const cha
> static void json_print_int(AVTextFormatContext *wctx, const char *key, int64_t value)
> {
> JSONContext *json = wctx->priv;
> - const struct AVTextFormatSection *parent_section = wctx->level ?
> - wctx->section[wctx->level-1] : NULL;
> + const AVTextFormatSection *parent_section = wctx->level ? wctx->section[wctx->level - 1] : NULL;
> AVBPrint buf;
>
> if (wctx->nb_item[wctx->level] || (parent_section && parent_section->flags & AV_TEXTFORMAT_SECTION_FLAG_NUMBERING_BY_TYPE))
> 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..29889598bb 100644
> --- a/fftools/textformat/tw_avio.c
> +++ b/fftools/textformat/tw_avio.c
> @@ -23,6 +23,7 @@
> #include <string.h>
>
> #include "avtextwriters.h"
> +#include "libavutil/avassert.h"
>
> #include "libavutil/error.h"
>
> @@ -53,7 +54,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 +79,14 @@ const AVTextWriter avtextwriter_avio = {
>
> 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;
>
> -
> ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
> if (ret < 0)
> return ret;
> @@ -103,6 +108,8 @@ int avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_fil
>
> int avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, int close_on_uninit)
> {
> + av_assert0(avio_ctx);
> +
> IOWriterContext *ctx;
> int ret;
In general I see some confusion about how to deal with invalid
parameters (this is probably an issue with the whole codebase), since
it's not always clear what's the right thing to do - consider that as
a programmer unrecoverable mistake and assert or let the user handle
that or just silently perform a no-op. It's probably fine to go with
the current patch and later refine in case we elaborate a set of
guidelines to apply.
_______________________________________________
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".