Re: [Qemu-devel] [PATCH] ensure all invocations to bdrv_{read, write} use (uint8_t *) for its third parameter

[Carlo Marcelo Arenas Belon]

On Fri, Jan 04, 2008 at 01:20:39PM +0000, Thiemo Seufer wrote:
> Carlo Marcelo Arenas Belon wrote:
> > Trivial fix that ensures that all buffers used for bdrv_read or bdrv_write
> > are from an array of the uint8_t type
> 
> Do we have a host where this actually makes a difference?

No that I know of (even if I'd heard horror stories about some bizarre old
architecure where sizeof(char) was 3 which I hope I never ever find),
and sorry for stirring this long thread by not being clear about my objective,
and not coming to clarify it before as I had no access until now to my email.

as you said this patch makes no difference in the logic at all in any
architecture that qemu uses (which is why I said it was a trivial fix)
but is IMHO a more correct version of the code because it is consistently
using the same type everywhere instead of different types with different names
in different places, even if they have the same storage requirements; after
all there is a reason why bdrv_read and bdrv_write where defined as using
(uint8_t *) for their buffer parameter since version 1.1 of the vl.h file.

I came to look for it when a merge conflict in kvm flipped the second
invocation from block.c into an "unsigned char *sector[512]" instead as you
can see by the proposed fix here :

  http://article.gmane.org/gmane.comp.emulators.kvm.devel/11510

Carlo

> > ---
> > Index: block-vvfat.c
> > ===================================================================
> > RCS file: /sources/qemu/qemu/block-vvfat.c,v
> > retrieving revision 1.16
> > diff -u -p -r1.16 block-vvfat.c
> > --- block-vvfat.c   24 Dec 2007 13:26:04 -0000      1.16
> > +++ block-vvfat.c   4 Jan 2008 07:57:20 -0000
> > @@ -340,7 +340,7 @@ typedef struct BDRVVVFATState {
> >      int current_fd;
> >      mapping_t* current_mapping;
> >      unsigned char* cluster; /* points to current cluster */
> > -    unsigned char* cluster_buffer; /* points to a buffer to hold temp data 
> > */
> > +    uint8_t* cluster_buffer; /* points to a buffer to hold temp data */
> >      unsigned int current_cluster;
> >  
> >      /* write support */
> > Index: block.c
> > ===================================================================
> > RCS file: /sources/qemu/qemu/block.c,v
> > retrieving revision 1.53
> > diff -u -p -r1.53 block.c
> > --- block.c 24 Dec 2007 16:10:43 -0000      1.53
> > +++ block.c 4 Jan 2008 07:57:21 -0000
> > @@ -459,7 +459,7 @@ int bdrv_commit(BlockDriverState *bs)
> >      BlockDriver *drv = bs->drv;
> >      int64_t i, total_sectors;
> >      int n, j;
> > -    unsigned char sector[512];
> > +    uint8_t sector[512];
> >  
> >      if (!drv)
> >          return -ENOMEDIUM;