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

Re: [PATCH 2/2] replace-object.c: remove the_repository from prepare_replace_object

From
Derrick Stolee <stolee@gmail.com>
Date
May 10, 2018, 14:53 UTC
Message-ID
<da155b13-b915-edb7-9bf9-6bef900cdde9@gmail.com>
In-Reply-To
<20180510115623.GA20632@sigill.intra.peff.net>
On 5/10/2018 7:56 AM, Jeff King wrote:
Show 15 quoted lines
> On Thu, May 10, 2018 at 07:23:13PM +0900, Junio C Hamano wrote:
>
>> This one was doing
>>
>> 	ptr = xmalloc(sizeof(*another_ptr))
>>
>> and it was OK because ptr and another_ptr happened to be of the same
>> type.  I wonder if we are making it safer, or making it more obscure
>> to seasoned C programmers, if we introduced a pair of helper macros,
>> perhaps like these:
>>
>> 	#define ALLOCATE(ptr) (ptr) = xmalloc(sizeof(*(ptr)))
>> 	#define CALLOCATE(ptr,cnt) (ptr) = xcalloc((cnt), sizeof(*(ptr)))
> I've often wondered that, too. It's the natural endgame of the
> ALLOC_ARRAY() road we've been going down.
I'll chime in that I like this idea.

Because I'm trying to learn more about Coccinelle, I added the diff below and ran 'make coccicheck'. It resulted in a 1000-line patch on 'next'. I'll refrain from sending that patch series, but it's nice to know a "simple" transformation is possible.

Using `git grep` I see 230 instances of 'xmalloc' and 261 instances of 'xcalloc'. After the Coccinelle transformation, these are down to 194 and 190, respectively, because the rest allocate in the same line as the definition. It's worth thinking about the macro pattern for those cases.

diff --git a/contrib/coccinelle/alloc.cocci b/contrib/coccinelle/alloc.cocci
new file mode 100644
index 0000000000..95f426c4dc
--- /dev/null
+++ b/contrib/coccinelle/alloc.cocci
@@ -0,0 +1,13 @@
+@@
+expression p;
+@@
+- p = xmalloc(sizeof(*p));
++ ALLOCATE(p);
+
+@@
+expression c;
+expression p;
+@@
+- p = xcalloc(c, sizeof(*p));
++ CALLOCATE(p,c);
+
diff --git a/git-compat-util.h b/git-compat-util.h
index f9e4c5f9bc..5398f217d7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -843,6 +843,9 @@ extern FILE *fopen_or_warn(const char *path, const 
char *mode);
   */
  #define FREE_AND_NULL(p) do { free(p); (p) = NULL; } while (0)

+#define ALLOCATE(ptr) (ptr) = xmalloc(sizeof(*(ptr)))
+#define CALLOCATE(ptr,cnt) (ptr) = xcalloc((cnt), sizeof(*(ptr)))
+
  #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), 
(alloc)))
  #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), 
st_mult(sizeof(*(x)), (alloc)))
Previous: Jeff KingNext: Stefan Beller
Message 5 of 8 in “object.c: free replace map in raw_object_store_clear”
  1. 1/2 object.c: free replace map in raw_object_store_clearStefan Beller, May 9, 2018
  2. 2/2 replace-object.c: remove the_repository from prepare_replace_objectStefan Beller, May 9, 2018
  3. Junio C HamanoMay 10, 2018
  4. Jeff KingMay 10, 2018
  5. Derrick StoleeMay 10, 2018
  6. Stefan BellerMay 10, 2018
  7. Junio C HamanoMay 10, 2018
  8. Stefan BellerMay 10, 2018

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.