Re: [hatari-devel] SCSI Driver improvements patch

[Uwe Seimet <Uwe.Seimet@xxxxxxxxx>]

I will definitely provide feedback, but this will take some time.

> Hi,
> 
> On 7.2.2025 21.16, Thomas Huth wrote:
> > Am Mon, 3 Feb 2025 19:21:38 +0100
> > schrieb Uwe Seimet <Uwe.Seimet@xxxxxxxxx>:
> >> any chance to get feedback on my question? Maybe what I am asking for is not
> >> possible or does not make sense. But my guess is that what I am supposed to
> >> change in the SCSI Driver code will result in code sequences that also exist
> >> elsewhere, or that other NatFeats code might also need.
> >> When I say "optionally" I do not mean something you configure with cmake,
> >> but something like a boolean parameter in the methods to be called.
> > 
> > For proper checking, you also need to know the size of the area that might
> > get accessed after getting the pointer. I guess there is no "one size fits
> > all" function that can be used everywhere, but we could at least add an
> > additional wrapper function that looks like this:
> > 
> > /**
> >   * Check if an area is valid and return a direct pointer to it in the
> >   * host memory (or NULL if it was invalid).
> >   */
> > void *STMemory_CheckedAreaPtr(uint32_t addr, int size, int mem_type)
> > {
> >         if (!STMemory_CheckAreaType(addr, size, mem_type ))
> >                 return NULL;
> > 
> >         return STMemory_STAddrToPointer(addr);
> > }
> > 
> > WDYT?
> 
> Sounds fine to me!
> 
> 
> 	- Eero
> 
> >>>> * read_stack_pointer(): calls STMemory_STAddrToPointer(), but does not
> >>>> check that the addresses are valid, as it should:
> >>>
> >>> Wouldn't it be better if Hatari was checking this in
> >>> STMemory_STAddrToPointer(), at least optionally? The existing conversions
> >>> between native and Atari address space already make the code in nf_scsidrv.c
> >>> (and likely elsewhere) hard to read. I don't doubt that these checks are
> >>> useful, but I wonder whether there are better solutions than explicit checks
> >>> everywhere.
> >>>
> >>> Best regards
> >>>
> >>> Uwe
> 
>