Libguestfs - Aug 2020 - Re: [nbdkit PATCH 3/5] api: Add nbdkit_string_intern helper

On 8/25/20 3:32 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 
> three (strdup, strndup, memdup) up front isn't hard.  If API 
> proliferation is a problem, you can combine strdup and strndup by 
> accepting -1 as a length to mean stop at NUL termination; but I'm less 
> worried about API proliferation and more about ease-of-use.
Time for some name bike-shedding.  How about:

nbdkit_strdup_intern (const char *)
nbdkit_strndup_intern (const char *s, size_t)

and if we need something that permits embedded NUL (right now I don't 
have such a use) we could add
nbdkit_memdup_intern (const void *s, size_t)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Wed, Aug 26, 2020 at 08:33:39AM -0500, Eric Blake
wrote:
> On 8/25/20 3:32 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 three (strdup, strndup, memdup) up front isn't hard.  If API
> >proliferation is a problem, you can combine strdup and strndup by
> >accepting -1 as a length to mean stop at NUL termination; but I'm
> >less worried about API proliferation and more about ease-of-use.
> 
> Time for some name bike-shedding.  How about:
> 
> nbdkit_strdup_intern (const char *)
> nbdkit_strndup_intern (const char *s, size_t)
> 
> and if we need something that permits embedded NUL (right now I
> don't have such a use) we could add
> nbdkit_memdup_intern (const void *s, size_t)
Seems good to me.  Do you think we'll be able to get the final APIs
into nbdkit this week?  I don't much care about finishing the
implementation, just that any APIs we add wont need to be changed
later.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On 8/26/20 8:35 AM, Richard W.M. Jones wrote:
>> Time for some name bike-shedding.  How about:
>>
>> nbdkit_strdup_intern (const char *)
>> nbdkit_strndup_intern (const char *s, size_t)
>>
>> and if we need something that permits embedded NUL (right now I
>> don't have such a use) we could add
>> nbdkit_memdup_intern (const void *s, size_t)
> 
> Seems good to me.  Do you think we'll be able to get the final APIs
> into nbdkit this week?  I don't much care about finishing the
> implementation, just that any APIs we add wont need to be changed
> later.
Yes.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org