mbox series

[0/4] grep: retire `init_grep_defaults()`

Message ID cover.1605972564.git.martin.agren@gmail.com (mailing list archive)
Headers show
Series grep: retire `init_grep_defaults()` | expand

Message

Martin Ågren Nov. 21, 2020, 6:31 p.m. UTC
Users of the grep machinery need to call `init_grep_defaults()` to
populate some defaults. There's a comment: "We could let the compiler do
this, but without C99 initializers the code gets unwieldy and 
unreadable, so...".

We have such initializers now, so we can simplify accordingly.

Martin Ågren (4):
  grep: don't set up a "default" repo for grep
  grep: use designated initializers for `grep_defaults`
  grep: simplify color setup
  MyFirstObjectWalk: drop `init_walken_defaults()`

 Documentation/MyFirstObjectWalk.txt | 34 +------------
 grep.h                              |  1 -
 builtin/grep.c                      |  1 -
 builtin/log.c                       |  1 -
 grep.c                              | 74 ++++++++++-------------------
 revision.c                          |  1 -
 6 files changed, 27 insertions(+), 85 deletions(-)

Comments

Johannes Schindelin Nov. 23, 2020, 11:03 a.m. UTC | #1
Hi Martin,

On Sat, 21 Nov 2020, Martin Ågren wrote:

> Users of the grep machinery need to call `init_grep_defaults()` to
> populate some defaults. There's a comment: "We could let the compiler do
> this, but without C99 initializers the code gets unwieldy and
> unreadable, so...".
>
> We have such initializers now, so we can simplify accordingly.

When I read this at first, I feared that you would change a
`pthread_init_mutex()` to using `PTHREAD_MUTEX_INITIALIZER` instead. On
the face of it, that would look good on paper, but it unfortunately falls
flat for us because there is no Win32 equivalent for the static
initializer: we emulate mutexes via `CRITICAL_SECTION`s, and those need to
be initialized by running a function.

To my surprise and relief, your patches do not do that; The only mutex in
`grep.c` (`grep_attr_mutex`) is not touched.

I just wanted to mention this here, in case anybody else had the same
idea.

Thanks,
Dscho