search for: read_cb

...pleIteratorStatus read_metadata_block_data_padding_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Seek seek_cb, FLAC__StreamMetadata_Padding *block, unsigned block_length); static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_application_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_Application *block, unsigned block_length); static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_SeekTable *block, unsigned block_length); -static FLAC__Metadata_SimpleIter...

...rc/libFLAC/metadata_iterators.c +++ b/src/libFLAC/metadata_iterators.c @@ -78,7 +78,7 @@ static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_padding_cb_( static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_application_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_Application *block, unsigned block_length); static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_SeekTable *block, unsigned block_length); static FLAC__Metadata_SimpleIter...

Janne Hyv?rinen wrote: > Patch v2, now handles more malformed cases. Original patch was for a > file for which I had a sample from a user but this allows handling some > manually broken test cases. Err, I'm getting warning messages on that patch: CC metadata_iterators.lo metadata_iterators.c: In function ?read_metadata_block_data_vorbis_comment_cb_?:

...pleIteratorStatus read_metadata_block_data_padding_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Seek seek_cb, FLAC__StreamMetadata_Padding *block, unsigned block_length); static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_application_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_Application *block, unsigned block_length); static FLAC__Metadata_SimpleIteratorStatus read_metadata_block_data_seektable_cb_(FLAC__IOHandle handle, FLAC__IOCallback_Read read_cb, FLAC__StreamMetadata_SeekTable *block, unsigned block_length); -static FLAC__Metadata_SimpleIter...

...ts marked [qemu-nbd] are thus dependent on * the fact that qemu-nbd is compliant. diff --git a/interop/structured-read.c b/interop/structured-read.c index 0b189d1..3e62b05 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -48,16 +48,13 @@ struct data { }; static int -read_cb (unsigned valid_flag, void *opaque, +read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, unsigned status, int *error) { struct data *data = opaque; const char *buf = bufv; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - /* The N...

...ed-read.c index f9014c8..d00524f 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -143,7 +143,7 @@ main (int argc, char *argv[]) memset (rbuf, 2, sizeof rbuf); data = (struct data) { .count = 2, }; - if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, &data, read_cb, + if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, read_cb, &data, 0) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); @@ -153,7 +153,7 @@ main (int argc, char *argv[]) /* Repeat with DF flag. */ memset (rb...

...p/structured-read.c index 0b189d1..31aadbe 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -147,7 +147,8 @@ main (int argc, char *argv[]) memset (rbuf, 2, sizeof rbuf); data = (struct data) { .count = 2, }; - if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, read_cb, &data, + if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, + (nbd_chunk_callback) { .callback = read_cb, .user_data = &data }, 0) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); @@...

..._data = buffer }, Worth splitting the long lines? > +++ b/interop/structured-read.c > @@ -147,7 +147,8 @@ main (int argc, char *argv[]) > > memset (rbuf, 2, sizeof rbuf); > data = (struct data) { .count = 2, }; > - if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, read_cb, &data, > + if (nbd_pread_structured (nbd, rbuf, sizeof rbuf, 2048, > + (nbd_chunk_callback) { .callback = read_cb, .user_data = &data }, > 0) == -1) { > fprintf (stderr, "%s\n", nbd_get_error ()); >...

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is

...ts marked [qemu-nbd] are thus dependent on * the fact that qemu-nbd is compliant. diff --git a/interop/structured-read.c b/interop/structured-read.c index 31aadbe..569a56f 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -48,16 +48,13 @@ struct data { }; static int -read_cb (unsigned valid_flag, void *opaque, +read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, unsigned status, int *error) { struct data *data = opaque; const char *buf = bufv; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - /* The N...

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

...g was passed to request */ + bool df; /* input: true if DF flag was passed to request */ int count; /* input: count of expected remaining calls */ bool fail; /* input: true to return failure */ bool seen_hole; /* output: true if hole encountered */ @@ -60,15 +60,24 @@ read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, switch (status) { case LIBNBD_READ_DATA: - // XXX if (df...) - assert (buf == rbuf + 512); - assert (count == 512); - assert (offset == 2048 + 512); - assert (buf[0] == 1 && memcmp (buf, buf + 1, 511) == 0...

...TO; + cmd->error = error ? error : EPROTO; } SET_NEXT_STATE(%FINISH); diff --git a/interop/structured-read.c b/interop/structured-read.c index cf8b893..46a7a80 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -47,7 +47,7 @@ struct data { static int read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, - int error, int status) + int *error, int status) { struct data *data = opaque; const char *buf = bufv; @@ -55,7 +55,7 @@ read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, /* The NBD spe...

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

...;ascii-string.h" #include "cleanup.h" @@ -73,6 +74,18 @@ static int debug_cb (CURL *handle, curl_infotype type, static size_t header_cb (void *ptr, size_t size, size_t nmemb, void *opaque); static size_t write_cb (char *ptr, size_t size, size_t nmemb, void *opaque); static size_t read_cb (void *ptr, size_t size, size_t nmemb, void *opaque); +static void lock_cb (CURL *handle, curl_lock_data data, + curl_lock_access access, void *userptr); +static void unlock_cb (CURL *handle, curl_lock_data data, + void *userptr); + +/* These locks protect...

...> +int > +nbd_unlocked_kill_command (struct nbd_handle *h, int signal) > +{ Is gcc -Wshadow going to annoy us by this choice of naming? I'd use signum, if we're worried. > @@ -149,6 +150,7 @@ main (int argc, char *argv[]) > read_cb, NULL, > completion_cb, NULL, 0); > if (cookie == -1) NBD_ERROR; > + nbd_kill_command (nbd, 0); This looks a bit funny until I read the docs at [1]. When using kill(2), I'm used to the function call 'kill(pid, 0)' probing for...

...ts marked [qemu-nbd] are thus dependent on * the fact that qemu-nbd is compliant. diff --git a/interop/structured-read.c b/interop/structured-read.c index 4087c95..4c6e619 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -48,12 +48,16 @@ struct data { }; static int -read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, +read_cb (unsigned valid_flag, void *opaque, + const void *bufv, size_t count, uint64_t offset, unsigned status, int *error) { struct data *data = opaque; const char *buf = bufv; + if (!(valid_flag & LIB...

...+{ Is it worth assert(!debug_fn_free), to prove that we never have use-after-free? > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + debug_fn_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) > + debug_fn_free++; > + return 0; > +} > + > +static int > +read_cb (unsigned valid_flag, void *opaque, > + const void *subbuf, size_t count, > + uint64_t offset, unsigned status, int *error) > +{ Same here. > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + read_cb_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) &g...

...imes.c +++ b/tests/closure-lifetimes.c @@ -116,9 +116,9 @@ main (int argc, char *argv[]) if (nbd == NULL) NBD_ERROR; if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR; - cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, - read_cb, NULL, - completion_cb, NULL, 0); + cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, + read_cb, NULL, + completion_cb, NULL, 0); if (cookie == -1) NBD_ERROR; assert...

..., "--exit-with-parent", "-v", "--filter=delay", "null", "size=512", - "delay-read=3", + "delay-read=10", NULL }; static unsigned debug_fn_valid; @@ -134,6 +134,7 @@ main (int argc, char *argv[]) assert (read_cb_free == 1); assert (completion_cb_free == 1); + nbd_kill_command (nbd, 0); nbd_close (nbd); /* Test command callbacks are freed if the handle is closed without @@ -149,6 +150,7 @@ main (int argc, char *argv[]) read_cb, NULL,...