git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Emily Shaffer <emilyshaffer@google.com>,
	Jeff King <peff@peff.net>
Subject: [PATCH v3 3/4] grep: copy struct in one fell swoop
Date: Sun, 29 Nov 2020 20:52:21 +0100
Message-ID: <77371dee2f13723cf8f766793b07241ca458adbd.1606679312.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1606679312.git.martin.agren@gmail.com>

We have a `struct grep_opt` with our defaults which we then copy into
the caller's struct. Rather than zeroing the target struct and copying
each element one by one, just copy everything at once. This leaves the
code simpler and more maintainable.

We don't have any ownership issues with what we're copying now and can
just greedily copy the whole thing. If and when we do need to handle
such elements (`char *`?), we must and can handle it appropriately. Make
sure to leave a comment to our future selves.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 grep.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/grep.c b/grep.c
index 8f2009ec9f..efeb6dc58d 100644
--- a/grep.c
+++ b/grep.c
@@ -66,11 +66,6 @@ static const char *color_grep_slots[] = {
 	[GREP_COLOR_SEP]	    = "separator",
 };
 
-static void color_set(char *dst, const char *color_bytes)
-{
-	xsnprintf(dst, COLOR_MAXLEN, "%s", color_bytes);
-}
-
 static int parse_pattern_type_arg(const char *opt, const char *arg)
 {
 	if (!strcmp(arg, "default"))
@@ -100,6 +95,14 @@ int grep_config(const char *var, const char *value, void *cb)
 	if (userdiff_config(var, value) < 0)
 		return -1;
 
+	/*
+	 * The instance of grep_opt that we set up here is copied by
+	 * grep_init() to be used by each individual invocation.
+	 * When populating a new field of this structure here, be
+	 * sure to think about ownership -- e.g., you might need to
+	 * override the shallow copy in grep_init() with a deep copy.
+	 */
+
 	if (!strcmp(var, "grep.extendedregexp")) {
 		opt->extended_regexp_option = git_config_bool(var, value);
 		return 0;
@@ -157,9 +160,6 @@ int grep_config(const char *var, const char *value, void *cb)
  */
 void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)
 {
-	struct grep_opt *def = &grep_defaults;
-	int i;
-
 #if defined(USE_LIBPCRE2)
 	if (!pcre2_global_context)
 		pcre2_global_context = pcre2_general_context_create(
@@ -171,26 +171,13 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix
 	pcre_free = free;
 #endif
 
-	memset(opt, 0, sizeof(*opt));
+	*opt = grep_defaults;
+
 	opt->repo = repo;
 	opt->prefix = prefix;
 	opt->prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
 	opt->pattern_tail = &opt->pattern_list;
 	opt->header_tail = &opt->header_list;
-
-	opt->only_matching = def->only_matching;
-	opt->color = def->color;
-	opt->extended_regexp_option = def->extended_regexp_option;
-	opt->pattern_type_option = def->pattern_type_option;
-	opt->linenum = def->linenum;
-	opt->columnnum = def->columnnum;
-	opt->max_depth = def->max_depth;
-	opt->pathname = def->pathname;
-	opt->relative = def->relative;
-	opt->output = def->output;
-
-	for (i = 0; i < NR_GREP_COLORS; i++)
-		color_set(opt->colors[i], def->colors[i]);
 }
 
 void grep_destroy(void)
-- 
2.29.2.454.gaff20da3a2


  parent reply	other threads:[~2020-11-29 19:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21 18:31 [PATCH 0/4] grep: retire `init_grep_defaults()` Martin Ågren
2020-11-21 18:31 ` [PATCH 1/4] grep: don't set up a "default" repo for grep Martin Ågren
2020-11-21 18:31 ` [PATCH 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren
2020-11-21 18:31 ` [PATCH 3/4] grep: simplify color setup Martin Ågren
2020-11-21 20:23   ` Jeff King
2020-11-21 20:52     ` Martin Ågren
2020-11-21 22:46     ` Junio C Hamano
2020-11-24  6:54       ` Jeff King
2020-11-21 18:31 ` [PATCH 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren
2020-11-23 11:03 ` [PATCH 0/4] grep: retire `init_grep_defaults()` Johannes Schindelin
2020-11-24 21:04 ` [PATCH v2 0/4] grep: simplify "grep defaults" handling Martin Ågren
2020-11-24 21:04   ` [PATCH v2 1/4] grep: don't set up a "default" repo for grep Martin Ågren
2020-11-24 21:04   ` [PATCH v2 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren
2020-11-24 21:04   ` [PATCH v2 3/4] grep: copy struct in one fell swoop Martin Ågren
2020-11-24 22:34     ` Junio C Hamano
2020-11-25  6:25       ` Martin Ågren
2020-11-25  7:53         ` Junio C Hamano
2020-11-26 20:25           ` Martin Ågren
2020-11-24 21:04   ` [PATCH v2 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren
2020-11-25  9:27   ` [PATCH v2 0/4] grep: simplify "grep defaults" handling Ævar Arnfjörð Bjarmason
2020-11-29 19:52   ` [PATCH v3 " Martin Ågren
2020-11-29 19:52     ` [PATCH v3 1/4] grep: don't set up a "default" repo for grep Martin Ågren
2020-11-29 19:52     ` [PATCH v3 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren
2020-11-29 19:52     ` Martin Ågren [this message]
2020-11-29 19:52     ` [PATCH v3 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren
2020-12-01  4:46     ` [PATCH v3 0/4] grep: simplify "grep defaults" handling Junio C Hamano

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=77371dee2f13723cf8f766793b07241ca458adbd.1606679312.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git