Re: [PATCH 1/1 linux-next] fs/coda/upcall.c: remove UPARG flow control macro

[Jan Harkes <jaharkes@cs.cmu.edu>]

On Mon, Apr 27, 2015 at 07:23:06PM +0200, Fabian Frederick wrote:
> Move UPARG code to alloc_upcall() and test errors/return in callsites.
> This patch removes flow control in macros which must be avoided.
> (See Documentation/CodingStyle)

At first glance this is not a correct patch.

UPARG allocates a buffer that is large enough to hold both the upcall
message going to userspace as well as the incoming reply message. At
first glance this patch only seems to allocate insize. It may still be
doing the right thing because insize is defined at the call level as
MAX(sizeof(inarg), sizeof(outarg)). This it looks like quite a few
changes for an untested patch and not enough for an actual cleanup of
the related code.

Jan

> ---
> This is untested.
> 
>  fs/coda/upcall.c | 107 +++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 69 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/coda/upcall.c b/fs/coda/upcall.c
> index 9b1ffaa..7524f630 100644
> --- a/fs/coda/upcall.c
> +++ b/fs/coda/upcall.c
> @@ -41,30 +41,24 @@
>  static int coda_upcall(struct venus_comm *vc, int inSize, int *outSize,
>  		       union inputArgs *buffer);
>  
> -static void *alloc_upcall(int opcode, int size)
> +static int alloc_upcall(int opcode, int insize, int *outsize,
> +			union inputArgs **inp, union outputArgs **outp)
>  {
> -	union inputArgs *inp;
> +	CODA_ALLOC(*inp, union inputArgs *, insize);
> +	if (!*inp)
> +		return -ENOMEM;
>  
> -	CODA_ALLOC(inp, union inputArgs *, size);
> -        if (!inp)
> -		return ERR_PTR(-ENOMEM);
...