Mailing List Archive

[PATCH libgpg-error] build: Be more consistent with memory management.
From: Érico Nogueira <erico.erc@gmail.com>

* src/mkheader.c: remove xfree wrapper (free(NULL) is well-defined),
use xmalloc instead of malloc+check return value, use macros to block
malloc and strdup from being used in the program.
---
src/mkheader.c | 35 +++++++++--------------------------
1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/mkheader.c b/src/mkheader.c
index 1d2ea20..995f95f 100644
--- a/src/mkheader.c
+++ b/src/mkheader.c
@@ -41,16 +41,7 @@ static int use_posix_threads;
static int stdint_h_included;
static int sys_types_h_included;

-
-/* The usual free wrapper. */
-static void
-xfree (void *a)
-{
- if (a)
- free (a);
-}
-
-
+/* Malloc wrappers. */
static char *
xmalloc (size_t n)
{
@@ -77,6 +68,9 @@ xstrdup (const char *string)
return p;
}

+/* Protect from using raw malloc or strdup in rest of file. */
+#define malloc undef
+#define strdup undef

/* Return a malloced string with TRIPLET. If TRIPLET has an alias
* return that instead. In general build-aux/config.sub should do the
@@ -161,7 +155,7 @@ canon_host_triplet (const char *triplet, int no_vendor_hack, char **r_os)
}
*p = 0;
result = canon_host_triplet (buf, 1, NULL);
- xfree (buf);
+ free (buf);
goto leave;
}

@@ -233,7 +227,7 @@ parse_config_h (const char *fname)
p1 = strtok (NULL, "\"");
if (!*p1)
continue; /* oops */
- xfree (replacement_for_off_type);
+ free (replacement_for_off_type);
replacement_for_off_type = xstrdup (p1);
}
else if (!strcmp (p1, "USE_POSIX_THREADS"))
@@ -364,14 +358,8 @@ mk_include_name (const char *name, const char *repl)
char *incfname, *p;
const char *s;

- incfname = malloc (strlen (srcdir) + strlen (name)
+ incfname = xmalloc (strlen (srcdir) + strlen (name)
+ (repl?strlen (repl):0) + 1);
- if (!incfname)
- {
- fputs (PGM ": out of core\n", stderr);
- exit (1);
- }
-
if (*name == '.' && name[1] == '/')
*incfname = 0;
else
@@ -676,12 +664,7 @@ main (int argc, char **argv)

host_triplet = canon_host_triplet (host_triplet_raw, 0, &host_os);

- srcdir = malloc (strlen (fname) + 2 + 1);
- if (!srcdir)
- {
- fputs (PGM ": out of core\n", stderr);
- return 1;
- }
+ srcdir = xmalloc (strlen (fname) + 2 + 1);
strcpy (srcdir, fname);
p1 = strrchr (srcdir, '/');
if (p1)
@@ -775,6 +758,6 @@ main (int argc, char **argv)
if (fp)
fclose (fp);

- xfree (host_triplet);
+ free (host_triplet);
return 0;
}
--
2.31.1


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
[PATCH libgpg-error] build: Be more consistent with memory management. [ In reply to ]
From: Érico Nogueira <erico.erc@gmail.com>

* src/mkheader.c: remove xfree wrapper (free(NULL) is well-defined),
use xmalloc instead of malloc+check return value, use macros to block
malloc and strdup from being used in the program.
---
src/mkheader.c | 35 +++++++++--------------------------
1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/mkheader.c b/src/mkheader.c
index 1d2ea20..995f95f 100644
--- a/src/mkheader.c
+++ b/src/mkheader.c
@@ -41,16 +41,7 @@ static int use_posix_threads;
static int stdint_h_included;
static int sys_types_h_included;

-
-/* The usual free wrapper. */
-static void
-xfree (void *a)
-{
- if (a)
- free (a);
-}
-
-
+/* Malloc wrappers. */
static char *
xmalloc (size_t n)
{
@@ -77,6 +68,9 @@ xstrdup (const char *string)
return p;
}

+/* Protect from using raw malloc or strdup in rest of file. */
+#define malloc undef
+#define strdup undef

/* Return a malloced string with TRIPLET. If TRIPLET has an alias
* return that instead. In general build-aux/config.sub should do the
@@ -161,7 +155,7 @@ canon_host_triplet (const char *triplet, int no_vendor_hack, char **r_os)
}
*p = 0;
result = canon_host_triplet (buf, 1, NULL);
- xfree (buf);
+ free (buf);
goto leave;
}

@@ -233,7 +227,7 @@ parse_config_h (const char *fname)
p1 = strtok (NULL, "\"");
if (!*p1)
continue; /* oops */
- xfree (replacement_for_off_type);
+ free (replacement_for_off_type);
replacement_for_off_type = xstrdup (p1);
}
else if (!strcmp (p1, "USE_POSIX_THREADS"))
@@ -364,14 +358,8 @@ mk_include_name (const char *name, const char *repl)
char *incfname, *p;
const char *s;

- incfname = malloc (strlen (srcdir) + strlen (name)
+ incfname = xmalloc (strlen (srcdir) + strlen (name)
+ (repl?strlen (repl):0) + 1);
- if (!incfname)
- {
- fputs (PGM ": out of core\n", stderr);
- exit (1);
- }
-
if (*name == '.' && name[1] == '/')
*incfname = 0;
else
@@ -676,12 +664,7 @@ main (int argc, char **argv)

host_triplet = canon_host_triplet (host_triplet_raw, 0, &host_os);

- srcdir = malloc (strlen (fname) + 2 + 1);
- if (!srcdir)
- {
- fputs (PGM ": out of core\n", stderr);
- return 1;
- }
+ srcdir = xmalloc (strlen (fname) + 2 + 1);
strcpy (srcdir, fname);
p1 = strrchr (srcdir, '/');
if (p1)
@@ -775,6 +758,6 @@ main (int argc, char **argv)
if (fp)
fclose (fp);

- xfree (host_triplet);
+ free (host_triplet);
return 0;
}
--
2.31.1


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH libgpg-error] build: Be more consistent with memory management. [ In reply to ]
On Tue, 27 Apr 2021 12:02, Érico Nogueira said:

> * src/mkheader.c: remove xfree wrapper (free(NULL) is well-defined),

It is weel defined but there SunOS bails out on it. Further on Windows
you need to use a matching free from the same CRT; in particular for
inter-DLL use malloc and free. Thuis having a warpper is a good idea.

> use xmalloc instead of malloc+check return value, use macros to block
> malloc and strdup from being used in the program.

Nope. xmalloc is a shortcut to make things easier for short living
programs; replacing existing code by its inferior variant is a Bad
Thing.

If you have a need for changes in libgpg-error, it is best to
communicate your reasons and not just to toss some patches.


Shalom-Salam,

Werner

--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
Re: [PATCH libgpg-error] build: Be more consistent with memory management. [ In reply to ]
Em 27/04/2021 15:34, Werner Koch escreveu:
> On Tue, 27 Apr 2021 12:02, Érico Nogueira said:
>
>> * src/mkheader.c: remove xfree wrapper (free(NULL) is well-defined),
>
> It is weel defined but there SunOS bails out on it. Further on Windows
> you need to use a matching free from the same CRT; in particular for
> inter-DLL use malloc and free. Thuis having a warpper is a good idea.

There were some free() calls in the code already. In that case, wouldn't
it be best to use a consistent function here (even if xfree())? Someone
testing only on modern *nix could miss the need to use xfree() in a
specific spot.

>
>> use xmalloc instead of malloc+check return value, use macros to block
>> malloc and strdup from being used in the program.
>
> Nope. xmalloc is a shortcut to make things easier for short living
> programs; replacing existing code by its inferior variant is a Bad
> Thing.

This is a build tool that doesn't really have much to do except fail
with an error if it can't allocate enough memory. Furthermore, the error
message + exit(1) or return 1; (in main()) code is needlessly duplicated
in the two spots where malloc() is used. Any further addition to the
tool would then have to either remember to use xmalloc() or copy the
error handling again. Blocking malloc() from being used means the
programmer (and reviewers) won't have to worry about remembering all the
rules. I agree that xmalloc doesn't fit in a library, but it definitely
fits in a build tool.

>
> If you have a need for changes in libgpg-error, it is best to
> communicate your reasons and not just to toss some patches.

I'm not tossing some patches, I just found a place to slightly improve
code terseness and overall consistency while I investigated a build
failure (for which I sent another patch).

>
>
> Shalom-Salam,
>
> Werner
>

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel