2016-02-01 Gustavo Padovan <gustavo@xxxxxxxxxxx>:
> Hi Maarten,
>
> 2016-02-01 Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>:
>
> > Op 29-01-16 om 22:20 schreef Gustavo Padovan:
> > > From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> > >
> > > Making fence_info a pointer enables us to extend the struct in the future
> > > without breaking the ABI.
> > >
> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/staging/android/sync.c | 2 +-
> > > drivers/staging/android/uapi/sync.h | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
> > > index f7530f0..51d4f47 100644
> > > --- a/drivers/staging/android/sync.c
> > > +++ b/drivers/staging/android/sync.c
> > > @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> > > if (info->status >= 0)
> > > info->status = !info->status;
> > >
> > > - len = sizeof(struct sync_file_info);
> > > + len = sizeof(struct sync_file_info) - sizeof(__u64 *);
> > >
> > > for (i = 0; i < sync_file->num_fences; ++i) {
> > > struct fence *fence = sync_file->cbs[i].fence;
> > > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h
> > > index ed281fc..9f07aa7 100644
> > > --- a/drivers/staging/android/uapi/sync.h
> > > +++ b/drivers/staging/android/uapi/sync.h
> > > @@ -54,7 +54,7 @@ struct sync_file_info {
> > > char name[32];
> > > __s32 status;
> > >
> > > - __u8 fence_info[0];
> > > + __u64 *fence_info;
> > > };
> > >
> > Pointers are awful, it should be a __u64 since it's a pointer type. Userspace should cast it to a uintptr_t in userspace.
>
> Oh, I made a mistake. I'll fix this.
>
>
> > This structure also won't work on 64-bits systems, there may be a hole between fence_info and status (or num_fences in next patch).
> >
> > It's probably best to move it to the top and ensure the struct is 64-bits aligned.
>
> That is not possible because we are not allocating only 64bits there but
> a array of struct fence_info, so it needs to be the last one. Maybe we
> can add some sort of padding?
Actually it is 64bits aligned:
struct sync_file_info {
char name[32];
__s32 status;
__u32 flags;
__u32 num_fences;
__u32 len;
__u64 fence_info;
};
So nothing to worry here.
Gustavo
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
