This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] getopt,argp - reentrant getopt interface, argp fixes


Hi,

[Note: I am not subscribed to bug-gnulib]

the attached three patches make the following changes (this is only an
overview, a precise changelog comes with each patch):

1. Add new reentrant interfaces for getopt (getopt_r, getopt_long_r,
   getopt_long_only_r) and supporting data type (struct getopt_data)
   and macro (GETOPT_DATA_INITIALIZER).
2. Change argp to use the new reentrant getopt interfaces.
3. Document the new interfaces.

Currently, argp uses a recursive lock (if available) to protect its
getopt invocations, and uses a mutex on GNU/Hurd.  Either is not
ideal.  Changing getopt to be reentrant is quite straightforward, and
solves the issue.

As a consequence it is now completely safe to invoke argp within an
argp option handler, for example to parse some of the options in a
dynamically loaded plugin.  This is just one example for a useful
applications of this (and in fact my original motivation).

My changes are conservative.  What follows are potential issues with
regards to reentrancy and design alternatives.

* Instead of or in addition to providing getopt_r etc, one could
  provide a new interface that takes as an additional argument a flag
  that indicates which style of option parsing should be used.

  My reason for not doing this is that the _r extension is a
  no-brainer, design and implementation wise.

  If you want me to do this, please provide me with a prototype that
  reflects what you want to be done, and I can make appropriate
  changes.

* getenv ("POSIXLY_CORRECT") is still called at initialization (for
  each "instance" of the parser).  What is needed is not the string
  content, but only the information "set" or "not set".  If this is
  not safe, one could add an argument to the prototype or a member to
  struct getopt_data that indicates if posix-correctness is desired.

  If you want me to do such a change, please indicate what's the
  prefered solution (a member in the struct or a function argument).
  This could be part of a more generic prototype (as indicated above).

* It is not safe to call getopt*_r on the same argument vector several
  times from different threads if reordering is enabled.  This is a
  documented limitation, and I don't think it can feasibly be avoided.

* If USE_NONOPTION_FLAGS is enabled (it is not, due to other problems
  with this concept), for each "instance" of the parser which is
  initialized it is checked if argc/argv is equal to
  __libc_argc/__libc_argv.  If yes, the nonoption flags as indicated
  by an environment variable that bash creates (created?)
  automatically are used to avoid interpreting wildcard expansions as
  options.

  This code is retained, and should still work as expected.  However,
  it emphasizes the limitation that no two instances of the parser
  must be created with the same argc/argv (which is
  __libc_argc/__libc_argv) concurrently.  The difference is that this
  limitation here even applies if you don't let getopt reorder the
  options (because otherwise there could be a race in the
  initialization when the nonoption flags are incorporated from global
  variables).

  This code is defunct, and the situation where an actual race could
  occur is a bizarre corner case.  I am not really sure how to deal
  with this in the best way.  Maybe nonoption flags should be disabled
  for the reentrant interface, or be provided as function arguments.
  Maybe you want to remove this code alltogether.

Please let me know what you think about this.  The patch has received
some testing by me and Marco Gerards.  In particular, I audited the
code for reentrancy issues and run a modified glibc on my main system.
Thanks also to Roland for suggesting this approach.

Thanks,
Marcus

Attachment: libc-getopt-r.diff
Description: Binary data

Attachment: libc-argp.diff
Description: Binary data

Attachment: libc-manual.diff
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]