A patch to move FSCTL handling into the VFS

[Rusty Russell]

On Fri, 16 Sep 2011 11:58:04 -0700, Richard Sharpe <realrichardsharpe at gmail.com> wrote:
> 2. Is the handling of talloc correct. Ie, will these allocations get
> cleaned up when the request completes. I used
> talloc_size(talloc_tos(), ...) mostly.

Yes, that looks wrong (I have no idea about this code, BTW, just that I
know something about talloc).

You usually want to use talloc_array, which gives you type checking:

> +static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle,
> +			      struct files_struct *fsp,
> +			      uint32_t function,
> +			      uint16_t req_flags,  /* Needed for UNICODE ... */
> +			      const char *in_data,
> +			      uint32_t in_len,
> +			      char **out_data,
> +			      uint32_t max_out_len,
> +			      uint32_t *out_len)
> +{
...
> +		*out_len = (max_out_len >= 64) ? 64 : max_out_len;
> +		/* Hmmm, will this cause problems if less data asked for? */
> +		return_data = talloc_size(talloc_tos(), 64);
> +		if (return_data == NULL) {
> +			return NT_STATUS_NO_MEMORY;
> +		}
> +
> +		/* For backwards compatibility only store the dev/inode. */
> +		push_file_id_16(return_data, &fsp->file_id);
> +		memcpy(return_data+16,create_volume_objectid(fsp->conn,objid),16);
> +		push_file_id_16(return_data+32, &fsp->file_id);
> +		*out_data = return_data;
> +		return NT_STATUS_OK;

This would be neater as an explicit char array allocation:
     return_data = talloc_array(<parent>, char, 64);

More importantly, the parent is wrong: you can't parent on the top of
stack then return!  Your function should take a MEM_CTX ptr: the caller
should pass the allocation which it wants the out_data to hang from.

> +		shadow_data = talloc_zero(talloc_tos(),
> +					    struct shadow_copy_data);
> +		if (shadow_data == NULL) {
> +			DEBUG(0,("TALLOC_ZERO() failed!\n"));
> +			reply NT_STATUS_NO_MEMORY;
> +		}

This is actually OK, since you only use this variable in this function.
> +		cur_pdata = talloc_size(talloc_tos(), *out_len);
> +		if (cur_pdata == NULL) {
> +			TALLOC_FREE(shadow_data);
> +			return NT_STATUS_NO_MEMORY;
> +		}
> +
> +		*out_data = cur_pdata;

This needs a mem_ctx to hang off...
> +		*out_len = 16;
> +		out_data_tmp = talloc_size(talloc_tos, *out_len);

Again...

Hope that helps,
Rusty.