git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/5] grep: store grep_source buffer as const

From
Jeff King <peff@peff.net>
Date
Sep 21, 2021, 03:51 UTC
Message-ID
<YUlWwOY7MD3KUyWh@coredump.intra.peff.net>
In-Reply-To
<YUlVZk1xXulAqdef@coredump.intra.peff.net>

Our grep_buffer() function takes a non-const buffer, which is confusing: we don't take ownership of nor write to the buffer.

This mostly comes from the fact that the underlying grep_source struct in which we store the buffer uses non-const pointer. The memory pointed to by the struct is sometimes owned by us (for FILE or OID sources), and sometimes not (for BUF sources).

Let's store it as const, which lets us err on the side of caution (i.e., the compiler will warn us if any of our code writes to or tries to free it).

As a result, we must annotate the one place where we do free it by casting away the constness. But that's a small price to pay for the extra safety and clarity elsewhere (and indeed, it already had a comment explaining why GREP_SOURCE_BUF _didn't_ free it).

And then we can mark grep_buffer() as taking a const buffer.
Signed-off-by: Jeff King <peff@peff.net>
---
 grep.c | 9 ++++++---
 grep.h | 4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/grep.c b/grep.c
index 9603ac119d..14fe8a0fd2 100644
--- a/grep.c
+++ b/grep.c
@@ -1820,9 +1820,10 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs)
 
 	return grep_source_1(opt, gs, 0);
 }
 
-static void grep_source_init_buf(struct grep_source *gs, char *buf,
+static void grep_source_init_buf(struct grep_source *gs,
+				 const char *buf,
 				 unsigned long size)
 {
 	gs->type = GREP_SOURCE_BUF;
 	gs->name = NULL;
@@ -1832,9 +1833,9 @@ static void grep_source_init_buf(struct grep_source *gs, char *buf,
 	gs->driver = NULL;
 	gs->identifier = NULL;
 }
 
-int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
+int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size)
 {
 	struct grep_source gs;
 	int r;
 
@@ -1884,9 +1885,11 @@ void grep_source_clear_data(struct grep_source *gs)
 {
 	switch (gs->type) {
 	case GREP_SOURCE_FILE:
 	case GREP_SOURCE_OID:
-		FREE_AND_NULL(gs->buf);
+		/* these types own the buffer */
+		free((char *)gs->buf);
+		gs->buf = NULL;
 		gs->size = 0;
 		break;
 	case GREP_SOURCE_BUF:
 		/* leave user-provided buf intact */
diff --git a/grep.h b/grep.h
index 128007db65..3cb8a83ae8 100644
--- a/grep.h
+++ b/grep.h
@@ -188,9 +188,9 @@ void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const
 void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
 void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
 void compile_grep_patterns(struct grep_opt *opt);
 void free_grep_patterns(struct grep_opt *opt);
-int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size);
+int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size);
 
 struct grep_source {
 	char *name;
 
@@ -201,9 +201,9 @@ struct grep_source {
 	} type;
 	void *identifier;
 	struct repository *repo; /* if GREP_SOURCE_OID */
 
-	char *buf;
+	const char *buf;
 	unsigned long size;
 
 	char *path; /* for attribute lookups */
 	struct userdiff_driver *driver;
-- 
2.33.0.1023.gc687d0d3c8
Previous: Jeff KingNext: Taylor Blau
Message 21 of 32 in “const-correctness in grep.c”
  1. 0/5 const-correctness in grep.cJeff King, Sep 21, 2021
  2. 1/5 grep: stop modifying buffer in strip_timestampJeff King, Sep 21, 2021
  3. Carlo ArenasSep 21, 2021
  4. Eric SunshineSep 21, 2021
  5. Carlo ArenasSep 21, 2021
  6. Jeff KingSep 21, 2021
  7. Carlo Marcelo Arenas BelónSep 21, 2021
  8. René ScharfeSep 21, 2021
  9. Jeff KingSep 21, 2021
  10. Ævar Arnfjörð BjarmasonSep 21, 2021
  11. Jeff KingSep 22, 2021
  12. Ævar Arnfjörð BjarmasonSep 23, 2021
  13. 2/5 grep: stop modifying buffer in show_line()Jeff King, Sep 21, 2021
  14. Taylor BlauSep 21, 2021
  15. Jeff KingSep 21, 2021
  16. Taylor BlauSep 21, 2021
  17. 3/5 grep: stop modifying buffer in grep_source_1()Jeff King, Sep 21, 2021
  18. 4/5 grep: mark "haystack" buffers as constJeff King, Sep 21, 2021
  19. Ævar Arnfjörð BjarmasonSep 21, 2021
  20. Jeff KingSep 21, 2021
  21. 5/5 grep: store grep_source buffer as constJeff King, Sep 21, 2021
  22. Taylor BlauSep 21, 2021
  23. Ævar Arnfjörð BjarmasonSep 21, 2021
  24. Jeff KingSep 21, 2021
  25. 6/5 grep.c: mark eol/bol and derived as "const char * const"Ævar Arnfjörð Bjarmason, Sep 21, 2021
  26. Jeff KingSep 21, 2021
  27. Ævar Arnfjörð BjarmasonSep 21, 2021
  28. Jeff KingSep 21, 2021
  29. Ævar Arnfjörð BjarmasonSep 23, 2021
  30. Junio C HamanoSep 24, 2021
  31. Junio C HamanoSep 22, 2021
  32. Junio C HamanoSep 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.