Re: [PATCH v2 06/11] staging/android: turn fence_info into a __u64 pointer

[Gustavo Padovan <gustavo@padovan.org>]

Hi Maarten,

2016-02-02 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:

> Op 02-02-16 om 14:23 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > Making fence_info a pointer enables us to extend the struct in the future
> > without breaking the ABI.
> >
> > v2: use type __u64 for fence_info
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  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..03b1214 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..8e2ed32 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;
> >
> With the information from the commit I would expect fence_info would be a true pointer.

I missed that. It is fixed for v3 now.

> 
> so if (sync_file_info->num_fences == 0) not copying fence_info, but set all members including this one.
> else for_all_fences() copy_to_user(fence_info struct)
>
> Otherwise you still can't extend the struct without breaking the abi.

all sync_files have at least 1 fence so we are left to the else case,
coupy all fences to the user. So we always have variable length at the
end of the struct. But I still don't see how this will prevent us from
breaking the API.

	Gustavo