Re: [PATCH 3/3] grep: avoid one strdup() per file

[Jeff King]

On Thu, Feb 15, 2018 at 10:56:15PM +0100, Rasmus Villemoes wrote:

> There is only one instance of grep_source_init(GREP_SOURCE_FILE), and in
> that case the path and identifier arguments are equal - not just as
> strings, but the same pointer is passed. So we can save some time and
> memory by reusing the gs->path = xstrdup_or_null(path) we have already
> done as gs->identifier, and changing grep_source_clear accordingly
> to avoid a double free.

IMHO this special case is not really worth it, unless we can show that
those few bytes saved somehow make a measurable difference in either
peak memory or execution speed.

-Peff

[PATCH 3/3] grep: avoid one strdup() per file

[Rasmus Villemoes]

There is only one instance of grep_source_init(GREP_SOURCE_FILE), and in
that case the path and identifier arguments are equal - not just as
strings, but the same pointer is passed. So we can save some time and
memory by reusing the gs->path = xstrdup_or_null(path) we have already
done as gs->identifier, and changing grep_source_clear accordingly
to avoid a double free.

Signed-off-by: Rasmus Villemoes 
---
 grep.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index 3d7cd0e96..b1532b1b6 100644
--- a/grep.c
+++ b/grep.c
@@ -1972,7 +1972,8 @@ void grep_source_init(struct grep_source *gs, enum 
grep_source_type type,
 
switch (type) {
case GREP_SOURCE_FILE:
-   gs->identifier = xstrdup(identifier);
+   gs->identifier = identifier == path ?
+   gs->path : xstrdup(identifier);
break;
case GREP_SOURCE_OID:
gs->identifier = oiddup(identifier);
@@ -1986,7 +1987,10 @@ void grep_source_init(struct grep_source *gs, enum 
grep_source_type type,
 void grep_source_clear(struct grep_source *gs)
 {
FREE_AND_NULL(gs->name);
-   FREE_AND_NULL(gs->path);
+   if (gs->path == gs->identifier)
+   gs->path = NULL;
+   else
+   FREE_AND_NULL(gs->path);
FREE_AND_NULL(gs->identifier);
grep_source_clear_data(gs);
 }
-- 
2.15.1