Skip to content

Commit

Permalink
wstream: Refactor wstream_new_buffer/wstream_write
Browse files Browse the repository at this point in the history
- Removed 'copy' parameter from `wstream_new_buffer`. Callers simply pass a
  copy of the buffer if required.
- Added a callback parameter, which is used to notify callers when the data is
  successfully written. The callback is also used to free the buffer(if
  required) and is compatible with `free` from the standard library.
  • Loading branch information
tarruda committed Jun 18, 2014
1 parent 0c764fb commit ac5fb40
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 18 deletions.
15 changes: 9 additions & 6 deletions src/nvim/os/channel.c
Expand Up @@ -194,9 +194,10 @@ static void parse_msgpack(RStream *rstream, void *data, bool eof)
// Perform the call
msgpack_rpc_call(channel->id, &unpacked.data, &response);
wstream_write(channel->data.streams.write,
wstream_new_buffer(channel->sbuffer->data,
wstream_new_buffer(xmemdup(channel->sbuffer->data,

This comment has been minimized.

Copy link
@oni-link

oni-link Jun 18, 2014

If wstream_write returns false nobody can free the WBuffer. Perhaps wstream_write should free the WBuffer
before returning false?

This comment has been minimized.

Copy link
@tarruda

tarruda Jun 18, 2014

Author Owner

👍

channel->sbuffer->size),
channel->sbuffer->size,
true));
free));

// Clear the buffer for future calls
msgpack_sbuffer_clear(channel->sbuffer);
Expand All @@ -218,9 +219,10 @@ static void parse_msgpack(RStream *rstream, void *data, bool eof)
"an object with high level of nesting",
&response);
wstream_write(channel->data.streams.write,
wstream_new_buffer(channel->sbuffer->data,
wstream_new_buffer(xmemdup(channel->sbuffer->data,
channel->sbuffer->size),
channel->sbuffer->size,
true));
free));
// Clear the buffer for future calls
msgpack_sbuffer_clear(channel->sbuffer);
}
Expand Down Expand Up @@ -310,9 +312,10 @@ static WBuffer *serialize_event(char *type, typval_T *data)
msgpack_packer packer;
msgpack_packer_init(&packer, &msgpack_event_buffer, msgpack_sbuffer_write);
msgpack_rpc_notification(event_type, event_data, &packer);
WBuffer *rv = wstream_new_buffer(msgpack_event_buffer.data,
WBuffer *rv = wstream_new_buffer(xmemdup(msgpack_event_buffer.data,
msgpack_event_buffer.size),
msgpack_event_buffer.size,
true);
free);
msgpack_rpc_free_object(event_data);
msgpack_sbuffer_clear(&msgpack_event_buffer);

Expand Down
2 changes: 1 addition & 1 deletion src/nvim/os/job.c
Expand Up @@ -258,7 +258,7 @@ bool job_write(int id, char *data, uint32_t len)
return false;
}

if (!wstream_write(job->in, wstream_new_buffer(data, len, false))) {
if (!wstream_write(job->in, wstream_new_buffer(data, len, free))) {
job_stop(job->id);
return false;
}
Expand Down
19 changes: 8 additions & 11 deletions src/nvim/os/wstream.c
Expand Up @@ -21,8 +21,9 @@ struct wstream {
};

struct wbuffer {
size_t refcount, size;
size_t size, refcount;
char *data;
wbuffer_data_finalizer cb;
};

typedef struct {
Expand Down Expand Up @@ -116,19 +117,16 @@ bool wstream_write(WStream *wstream, WBuffer *buffer)
///
/// @param data Data stored by the WBuffer
/// @param size The size of the data array
/// @param copy If true, the data will be copied into the WBuffer
/// @param cb Pointer to function that will be responsible for freeing
/// the buffer data(passing 'free' will work as expected).
/// @return The allocated WBuffer instance
WBuffer *wstream_new_buffer(char *data, size_t size, bool copy)
WBuffer *wstream_new_buffer(char *data, size_t size, wbuffer_data_finalizer cb)
{
WBuffer *rv = xmalloc(sizeof(WBuffer));
rv->size = size;
rv->refcount = 0;

if (copy) {
rv->data = xmemdup(data, size);
} else {
rv->data = data;
}
rv->cb = cb;
rv->data = data;

return rv;
}
Expand All @@ -141,8 +139,7 @@ static void write_cb(uv_write_t *req, int status)
data->wstream->curmem -= data->buffer->size;

if (!--data->buffer->refcount) {
// Free the data written to the stream
free(data->buffer->data);
data->buffer->cb(data->buffer->data);
free(data->buffer);
}

Expand Down
1 change: 1 addition & 0 deletions src/nvim/os/wstream_defs.h
Expand Up @@ -3,6 +3,7 @@

typedef struct wbuffer WBuffer;
typedef struct wstream WStream;
typedef void (*wbuffer_data_finalizer)(void *data);

#endif // NVIM_OS_WSTREAM_DEFS_H

0 comments on commit ac5fb40

Please sign in to comment.