bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: Bruno Haible <bruno@clisp.org>,
	 bug-gnulib@gnu.org,  Karl Berry <karl@freefriends.org>
Subject: Re: scratch_buffer.h, scratch_buffer_dupfree.c sync
Date: Thu, 03 Nov 2022 20:37:08 +0100	[thread overview]
Message-ID: <87o7tnesnv.fsf@oldenburg.str.redhat.com> (raw)
In-Reply-To: <2f9a6dd4-ebba-f452-6f30-635515f8c49f@cs.ucla.edu> (Paul Eggert's message of "Thu, 3 Nov 2022 11:26:45 -0700")

* Paul Eggert:

> What problems do you see with the interfaces, and are there efforts to
> come up with a better API? The need is there in GNU apps, each of
> which tends to roll its own code here.

dynarray has an aliasing violation in its implementation.  The embedded
pointer should be void * (not DYNARRAY_ELEMENT *), with casts added in
the generated code, so that the out-of-line code can use the correct
types.

Defining the element free function has a trap for the unwary programmer:
it's easy to miss the required pointer indirection.  I don't know a good
fix for that.

I view dynarray as a stop-gap until we can use a C++ template inside
glibc.  We won't be able to use std::vector because of gaps in memory
allocation failure handling, but application code interested in basic
type-generic data structures should really use libstdc++ and not
preprocessor hacks like dynarray.  With such wide use of xmalloc &
friends, the memory allocation issue would impact few applications.

The scratch buffer interface is mainly intended as a helper for calling
NSS functions because of their highly problematic buffer interface: it's
not just that the caller allocates, the callee does not provide any size
hint at all if the provided buffer is not large enough.  So the only
thing you can do is keep retrying with larger and larger buffers.  No
one should create interfaces like that.

One could argue that scratch buffers are needed because the NSS
interfaces exist today, but from a glibc perspective, I think we should
provide replacements for the problematic NSS functions that are
significantly easier to use, rather than relying on developers to put
something together using scratch buffers.  Maybe it's sufficient to make
the original functions like getpwuid thread-safe, although such
functions manipulating global state wouldn't be really library-safe.
(You wouldn't want to call getpwuid in a library because it might
clobber another getpwuid result still in use elsewhere, but that's an
issue that is present with or without thread safety.)

I added scratch_buffer_grow_preserve and scratch_buffer_set_array_size
as an afterthought.  Conceptually, they are unrelated to the NSS usage.
They predate dynarrays.  If dynarrays were easier to define, we should
have switched over to them because they have implicit overflow checking
for allocation sizes.

Thanks,
Florian



  reply	other threads:[~2022-11-03 19:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-02 22:37 scratch_buffer.h, scratch_buffer_dupfree.c sync Karl Berry
2022-11-03  1:18 ` Paul Eggert
2022-11-03  2:37   ` Bruno Haible
2022-11-03  3:09     ` Paul Eggert
2022-11-03 10:51       ` Bruno Haible
2022-11-03 11:03         ` Florian Weimer
2022-11-03 12:41           ` Bruno Haible
2022-11-03 18:27             ` Paul Eggert
2022-11-03 20:44               ` Bruno Haible
2022-11-03 18:26           ` Paul Eggert
2022-11-03 19:37             ` Florian Weimer [this message]
2022-11-03 20:40             ` Karl Berry
2022-11-03 21:12               ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o7tnesnv.fsf@oldenburg.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=bruno@clisp.org \
    --cc=bug-gnulib@gnu.org \
    --cc=eggert@cs.ucla.edu \
    --cc=karl@freefriends.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).