Re: [fuse-devel] validity of callback arguments

[Goswin v. B. <gos...@we...>]

Miklos Szeredi <mi...@sz...> writes:

> On Wed, 04 Mar 2009, Goswin von Brederlow wrote:
>> fuse_req_t could have a pointer to the raw buffer, fuse_file_info and
>> flock struct and all 3 are heap allocated (and anything else allocated
>> for a request). The fuse_reply_* function could free the 3 pointers if
>> != NULL. That would keep it inside libfuse and extend the argument
>> validity till a request is replied to.
>> 
>> It would mean though that fuse_session_ops->process() would change
>> ABI. It would take over ownership of the buf argument.
>> fuse_session_process() would obviously follow. But I think those two
>> are the only functions that would change.
>
> I'd really like to avoid an API/ABI change...

The major problem currently is that one can not pass the raw buffer
for the request to the callback. If the request had a field for the
raw buffer and one could querry that then one could do the following:

1) malloc() the request buffer and call fuse_session_process() with it.
2) in callbacks:
   const char *raw = fuse_raw_buffer(req);
   int res = fuse_reply_whatever(req);
   free(raw);

That would only work though with a custom main loop. The ABI/API
changes I proposed would also work for the existing main loops.


Would it be so bad to have

#if FUSE_USE_VERSION < 27
#  define fuse_session_ops fuse_session_ops_compat26
#  define fuse_session_process fuse_session_process_compat26

defines?

>> For the case of the write functions there could then be a
>> 
>> /**
>>  * Take over ownership of raw buffer of a request.
>>  *
>>  * @param req request we want to take over
>>  * @return the raw buffer underlying the request
>>  *
>>  * Note: The buffer must remain valid till the request is replied to
>>  * and must be freeed by the application after this call. This call
>>  * can only be called once on a request.
>>  */
>> const char * fuse_take_buffer(fuse_req_t req);
>> 
>> This could simply set the req->buf = NULL to stop libfuse from freeing
>> the buffer.
>
> I like this one.  This could be done without changing the existing
> API, yet allow programs to use the write buffer after the function
> call.

But that requires that the request is in control of freeing the
buffer. Currently the caller of fuse_session_ops->process() or
fuse_session_process() has control of that.

>> Unless you see something wrong with that I think I will play around
>> with writing a patch for that.
>
> I see no problem with this.
>
>> PS: How much overhead does the raw buffer have over the agrument
>> passed to the write callback? Will it be much bigger?
>
> The raw buffer has a fixed size (132kB), but the size of the argument
> varies according to what the request was, etc...  The raw buffer is
> sized so the larges request should fit into it.

Does that mean with 4MB read/writes the buffer will allways be 4MB?

> Thanks,
> Miklos

MfG
        Goswin