search for: nbdkit_string_intern

...which return const char * already intern'd. Signed-off-by: Eric Blake <eblake@redhat.com> --- See the cover letter: if we like this, there are several other plugins that could also reduce their .unload, or maybe we want to add convenience functions to make the combo 'nbdkit_realpath/nbdkit_string_intern' simpler. plugins/file/file.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/plugins/file/file.c b/plugins/file/file.c index 08418194..6a0aad93 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -64,8 +64,8 @@ #define fdatasync fsync #en...

...gt; function. However be careful when modifying strings because for some methods (eg. C<.config>) the plugin may save the string pointer that you pass along. So you may have to ensure that the string is not freed for the -lifetime of the server. +lifetime of the server; you may find C<nbdkit_string_intern> helpful +for avoiding a memory leak while still obeying lifecycle constraints. Note that if your filter registers a callback but in that callback it doesn't call the C<next> function then the corresponding method in the @@ -450,8 +451,10 @@ requires write access to the underlying d...

...PM, Eric Blake wrote: >>> -      keys[optind] = strndup (argv[optind], n); >>> -      if (keys[optind] == NULL) { >>> -        perror ("strndup"); >>> +      CLEANUP_FREE char *key = strndup (argv[optind], n); >>> +      const char *safekey = nbdkit_string_intern (key); >>> +      if (safekey == NULL) >>>           exit (EXIT_FAILURE); >> >> It seems it might also be nice to have a "strndup" version of the >> intern function. > > Or even a memdup.  Yeah, those are possibilities as well.  Doing all > t...

...t; > >>>-      keys[optind] = strndup (argv[optind], n); > >>>-      if (keys[optind] == NULL) { > >>>-        perror ("strndup"); > >>>+      CLEANUP_FREE char *key = strndup (argv[optind], n); > >>>+      const char *safekey = nbdkit_string_intern (key); > >>>+      if (safekey == NULL) > >>>          exit (EXIT_FAILURE); > >> > >>It seems it might also be nice to have a "strndup" version of the > >>intern function. > > > >Or even a memdup.  Yeah, those are possibilitie...

On Tue, Aug 25, 2020 at 10:46:57AM -0500, Eric Blake wrote: > + const char *nbdkit_string_intern (const char *str); > + > +Returns a copy of str, so that the caller may reclaim str and use the > +copy in its place. If the copy is created outside the scope of a > +connection (such as during C<.load>), the lifetime of the copy will > +last at least through C<.unload>;...

On 8/25/20 2:21 PM, Richard W.M. Jones wrote: > On Tue, Aug 25, 2020 at 10:46:57AM -0500, Eric Blake wrote: >> + const char *nbdkit_string_intern (const char *str); >> + >> +Returns a copy of str, so that the caller may reclaim str and use the >> +copy in its place. If the copy is created outside the scope of a >> +connection (such as during C<.load>), the lifetime of the copy will >> +last at least throu...

More patches on the way for improving .list_exports signature and adding .export_description, but this is the promised code showing why nbdkit_string_intern is useful. Patch 4 is somewhat RFC: we could either add new API to take the boilerplate from: foo_config(const char *key, const char *value) { if (strcmp (key, "file") == 0) { CLEANUP_FREE char *tmp = nbdkit_realpath (value); filename = nbdkit_string_intern (tmp); if (!fil...

...;s an actual problem here, because there is still a connection object inside the server any time we have a TCP connection, so you can just store it there, unless I'm misunderstanding something. But anyway ... > So I'm thinking I need to add a helper > function: > > const char *nbdkit_string_intern (const char *str); > > Return a pointer to a copy of str, where nbdkit owns the lifetime of > the copy (allowing the caller to not have to worry about persisting > str indefinitely). If called when there is no client connection > (such as during .load), the copy will remain valid t...

Use the recently added nbdkit_string_intern to make this possible. Testsuite coverage is added in both tls-fallback as well as a cleanup to test-eval-exports.sh. Signed-off-by: Eric Blake <eblake@redhat.com> --- plugins/eval/nbdkit-eval-plugin.pod | 2 ++ plugins/sh/nbdkit-sh-plugin.pod | 20 +++++++++-- plugins/sh/methods.h...

...except through public APIs ;) I've got the patch working (it passes 'make check-valgrind'), so I'll post it later this morning to demo why it is useful. > But anyway ... > >> So I'm thinking I need to add a helper >> function: >> >> const char *nbdkit_string_intern (const char *str); >> >> Return a pointer to a copy of str, where nbdkit owns the lifetime of >> the copy (allowing the caller to not have to worry about persisting >> str indefinitely). If called when there is no client connection >> (such as during .load), the copy...

...dle to store this string in, we would have a memory leak: there is no way to associate this inside the handle's struct so that .close can reclaim it, but storing it globally is not thread-safe to parallel client connections. So I'm thinking I need to add a helper function: const char *nbdkit_string_intern (const char *str); Return a pointer to a copy of str, where nbdkit owns the lifetime of the copy (allowing the caller to not have to worry about persisting str indefinitely). If called when there is no client connection (such as during .load), the copy will remain valid through .unload; if cal...

...#39;ve got the patch working (it passes 'make check-valgrind'), so > I'll post it later this morning to demo why it is useful. > > >But anyway ... > > > >>So I'm thinking I need to add a helper > >>function: > >> > >>const char *nbdkit_string_intern (const char *str); > >> > >>Return a pointer to a copy of str, where nbdkit owns the lifetime of > >>the copy (allowing the caller to not have to worry about persisting > >>str indefinitely). If called when there is no client connection > >>(such as dur...

I'm about to add an 'exportname' filter, and in the process, I noticed a few shortcomings in our API. Time to fix those before the 1.22 release locks our API in stone. First, .list_exports needs to know if it is pre- or post-TLS, as that may affect which names are exported. Next, overloading .list_exports to do both NBD_OPT_LIST and mapping "" to a canonical name is