Re: [FFmpeg-devel] [PATCH v2 02/10] fftools/textformat: Quality improvements

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> softworkz .
> Sent: Mittwoch, 16. April 2025 13:33
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2 02/10] fftools/textformat:
> Quality improvements
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Andreas Rheinhardt
> > Sent: Mittwoch, 16. April 2025 12:49
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2 02/10] fftools/textformat:
> > Quality improvements
> >
> > softworkz:
> > > From: softworkz <softworkz@hotmail.com>
> > >
> > > 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 811b14b999..4c8def8e65 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))))) {
> > >          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);
> >
> > We use -Wno-pointer-sign in order to avoid these ugly casts.
> 
> Yep, I know. Since I'm not a C-for-life developer, I'm using and
> taking attention of warnings and hints like clang-tidy.
> When you have a file with dozens of warnings these things are not
> helpful, because you cannot go through all of them a hundred times.
> When disabling certain warnings altogether, nothing is won, because
> often there's one in ten cases where the it hints at a problem while
> all others are harmless.
> In order to get rid of a warning, you can either add ugly comments
> or - the designated way from the compiler side is to be explicit by
> e.g. making explicit casts. That's why you see them at some places.
> Doing so, improves quality when working - even for languages that
> I know in a similar way like you know C.
> 
> If we could agree to remove these in a future commit, it would be
> great. 

Hi Andreas,

as my comment above sounds like a typical never-gonna-happen story
I have prepared a commit for later removal of all redundant casts:

https://github.com/softworkz/FFmpeg/tree/submit_textformat_remove_casts

Thanks,
sw






_______________________________________________
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".