Re: CVS commit: src/sys/arch/sh3/include

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Tue, Sep 30, 2025 at 01:37:17 +0300, Valery Ushakov wrote:

> On Mon, Sep 29, 2025 at 20:08:54 +0000, Roland Illig wrote:
> 
> > Module Name:	src
> > Committed By:	rillig
> > Date:		Mon Sep 29 20:08:54 UTC 2025
> > 
> > Modified Files:
> > 	src/sys/arch/sh3/include: dmacreg.h
> > 
> > Log Message:
> > sh3/dmacreg.h: fix snprintb definitions
> > 
> > In the previous definitions, the 'f' conversion was combined with the
> > ':' conversion, which snprintb rejects at runtime, starting with
> > snprintb.c 1.47 from 2024-04-07.
> 
> I really would have appreciated a courtesy heads up...
> 
> I completely forgot what was the issue with F/f.  The claim in the
> commit message that changed snprintb was:
> 
>   These combinations would lead to garbled output.
> 
> The old format prints (libutil on 10) for SH3_DMAC_CMT_CMCSR_BITS e.g
> 
>   0x80<CMF,CKS=0(1/4)>
> 
> and the new one prints
> 
>   0x80<CMF,CKS=0=1/4>
> 
> What exactly about the first (old) one, that combines f and : is
> garbled?  I detest the latter quite intensly ("0=1/4" is both bogus
> and the whole thing looks ugly with those two equal signs).

Being forced to look at this anyway, I think the original change must
be reverted before 11 is out.  It prevents nice things, e.g

  #include <stdio.h>
  #include <util.h>

  // the original (f and :)
  #define SH3_DMAC_CMT_CMCSR_BITS_VERBOSE "\177\20"                       \
          "b\7CMF\0"                                                      \
          "f\0\2CKS\0" ":\0(1/4)\0" ":\1(1/8)\0" ":\2(1/16)\0" ":\3(1/64)\0"

  // with F and =
  #define SH3_DMAC_CMT_CMCSR_BITS_TERSE "\177\20" \
          "b\7CMF\0"                              \
          "F\0\2CKS\0" "*CKS\0"                   \
                  "=\0" "1/4\0"  "=\1" "1/8\0"    \
                  "=\2" "1/16\0" "=\3" "1/64\0"

  int
  main()
  {
      char buf[128];

      snprintb(buf, sizeof(buf), SH3_DMAC_CMT_CMCSR_BITS_VERBOSE, 0x80);
      printf("%s\n", buf);

      snprintb(buf, sizeof(buf), SH3_DMAC_CMT_CMCSR_BITS_TERSE, 0x80);
      printf("%s\n", buf);

      return 0;
  }

that produces

  0x80<CMF,CKS=0(1/4)>
  0x80<CMF,CKS=1/4>

But on current it produces instead an obnoxious, "you are not too
smart to use this function correctly, are you" output:

  0x80<CMF,CKS=0#
  0x80<CMF,CKS#

that is literally garbled.

Now, bugs happen.  Unlike gcc warnings about printf format mismatch
that are presented to the author of the code, the above will be shown
to a user that was told to run something with debug enabled, but,
alas, the author made a mistake and now snprintb's refusal to deal
with it b/c of abstract truth might lead itstead to a very real blues.
That error condition that is so hard to reproduce hit, but b/c of a
mistake in the snprintb format it's all in vain...


-uwe