vfs_btrfs: fix compile on 32-bit platforms and the issues with getting simple changes reviewed.

Andrew Bartlett abartlet at samba.org
Sat Mar 23 03:48:31 MDT 2013


On Sat, 2013-03-23 at 09:40 +0100, Rusty Russell wrote:

> commit fd6d0361d6fef5f8175967ddbae4a2b1d79dfcad
> Author: Rusty Russell <rusty at rustcorp.com.au>
> Date:   Sat Mar 23 17:26:57 2013 +1030
> 
>     vfs_btrfs: fix compile on 32-bit platforms.
>     
>     uint64_t are not unsigned longs on 32-bit platforms:
>     
>     [3265/3996] Compiling source3/modules/vfs_btrfs.c
>     ../source3/modules/vfs_btrfs.c: In function ‘btrfs_copy_chunk_send’:
>     ../source3/modules/vfs_btrfs.c:118:3: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t’ [-Werror=format]
>     ../source3/modules/vfs_btrfs.c:118:3: error: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘int64_t’ [-Werror=format]
>     ../source3/modules/vfs_btrfs.c:118:3: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Werror=format]
>     ../source3/modules/vfs_btrfs.c:118:3: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘uint64_t’ [-Werror=format]
>     ../source3/modules/vfs_btrfs.c: In function ‘btrfs_copy_chunk_recv’:
>     ../source3/modules/vfs_btrfs.c:180:2: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘off_t’ [-Werror=format]
>     cc1: some warnings being treated as errors
>     
>     Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

Rusty,

This looks wrong, and I had an alternative patch awaiting review on the
mailing list since Wednesday.  In particular the difference is that we
cast to long long, and change the format string, because these
parameters can be > 4GB, particularly the offset.

Perhaps revert this, and merge the patch I was waiting for review on?

I note with irony that this shows up the worst of our current voluntary
review situation.  I'm trying hard to ask for reviews, but they are hard
to come by, even for simple build errors.  I probably should have taken
metze's comments as a positive review, or have asked for a
clarification, but in the meantime you and others remain put out.  That
in turn made you put this in without review - because it was getting in
your way, and reviews are voluntary - and we need to go over it again. 

> diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
> index 660bc68..eed5456 100644
> --- a/source3/modules/vfs_btrfs.c
> +++ b/source3/modules/vfs_btrfs.c
> @@ -117,9 +117,9 @@ static struct tevent_req *btrfs_copy_chunk_send(struct vfs_handle_struct *handle
>  		 */
>  		DEBUG(5, ("BTRFS_IOC_CLONE_RANGE failed: %s, length %lu, "
>  			  "src fd: %ld off: %lu, dest fd: %d off: %lu\n",
> -			  strerror(errno), cr_args.src_length,
> -			  cr_args.src_fd, cr_args.src_offset,
> -			  dest_fsp->fh->fd, cr_args.dest_offset));
> +			  strerror(errno), (long)cr_args.src_length,
> +			  (long)cr_args.src_fd, (long)cr_args.src_offset,
> +			  dest_fsp->fh->fd, (long)cr_args.dest_offset));
>  		cc_state->subreq = SMB_VFS_NEXT_COPY_CHUNK_SEND(handle,
>  								cc_state, ev,
>  								src_fsp,
> @@ -177,7 +177,8 @@ static NTSTATUS btrfs_copy_chunk_recv(struct vfs_handle_struct *handle,
>  		return status;
>  	}
>  
> -	DEBUG(10, ("server side copy chunk copied %lu\n", cc_state->copied));
> +	DEBUG(10, ("server side copy chunk copied %lu\n",
> +		   (long)cc_state->copied));
>  	*copied = cc_state->copied;
>  	tevent_req_received(req);
>  	return NT_STATUS_OK;
> 
> 

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org




More information about the samba-technical mailing list