Re: [hatari-devel] Patch for bits

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Am Sun, 14 Apr 2024 10:13:20 +0200
schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:

> > Am 14.04.2024 um 09:47 schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:
> > 
> > Le 14/04/2024 à 07:52, Andreas Grabher a écrit :  
> >> I don’t think this patch is a good solution.
> >> https://git.tuxfamily.org/hatari/hatari.git/commit/?id=943006bc662e8ad7a0b364ff3b91907f40523303 <https://git.tuxfamily.org/hatari/hatari.git/commit/?id=943006bc662e8ad7a0b364ff3b91907f40523303>
> >> It just makes things complicated. Would’t it be easier to just rename the bits variable in scc.c? For example:
> >> static voidSCC_Update_RR0_Clear ( int Channel , int clear_bits )
> >> {
> >> SCC.Chn[ Channel ].RR0_No_Latch &= ~clear_bits;
> >> }
> >> static voidSCC_Update_RR0_Set ( int Channel , int set_bits )
> >> {
> >> SCC.Chn[ Channel ].RR0_No_Latch |= set_bits;
> >> }  
> > 
> > Hi
> > 
> > At first, I wanted to rename all "bits" variables in Hatari, but there're quite a lot over the files and also I don't see why  code in Hatari (excluding cpu core) should be dependant of some variables names that are used only for the cpu core, so I'd rather disable these enum when outside of the cpu core.
> > That's a little hack, but I think it's better than renaming everything and adding in our docs that "bits" is now some kind of reserved word that can't be used anymore to name a variable.
> > 
> > other method could be to split readcpu.h in 2 files and only include the file that don't declare the enum's in hatari's files, but that would create more differences with WinUAE's cpu core, which doesn't make things easier to keep track of changes in WinUAE.
> > 
> > Nicolas
> >   
> I don’t see many uses of bits outside the CPU core. I think it would be much cleaner to just rename them or leave it as is. It is just a warning (I think it is enabled by -Wshadow=local flag) and there are no impacts on functionality anyway. Adding compile-time checks that resolve to true of false depending on where a file is included from can cause bugs that are very hard to find.

IMHO the proper solution would be to rename the entries in the enum in
readcpu.h and submit that patch to Toni. Having a definition named "bits"
in a header that is included all over the place sounds very fragile to me
otherwise.

 Thomas



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/