[Linuxptp-devel] [PATCH 0/3] Terminate timemaster when memory allocation fails

[Miroslav Lichvar]

As per the discussion on linuxptp-users, this is a set that tries to
fix the unhandled errors in memory allocation in timemaster.

Miroslav Lichvar (3):
util: add wrappers for memory allocation functions.
util: exit in string_* and parray_* functions when allocation fails.
timemaster: use wrapped memory allocation functions.

timemaster.c | 82 ++++++++++++++++++++++++++++++------------------------------
util.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++-----------
util.h | 67 ++++++++++++++++++++++++++++++++++++++++---------
3 files changed, 161 insertions(+), 67 deletions(-)

[Miroslav Lichvar]

---
util.c | 27 +++++++++++++--------------
util.h | 30 ++++++++++++++++++------------
2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/util.c b/util.c
index 284657b..e7245d6 100644
--- a/util.c
+++ b/util.c
@@ -426,8 +426,10 @@ char *string_newf(const char *format, ...)
char *s;

va_start(ap, format);
- if (vasprintf(&s, format, ap) < 0)
- s = NULL;
+ if (vasprintf(&s, format, ap) < 0) {
+ pr_err("failed to allocate memory");
+ exit(1);
+ }
va_end(ap);

return s;
@@ -439,9 +441,8 @@ void string_append(char **s, const char *str)

len1 = strlen(*s);
len2 = strlen(str);
- *s = realloc(*s, len1 + len2 + 1);
- if (*s)
- memcpy((*s) + len1, str, len2 + 1);
+ *s = xrealloc(*s, len1 + len2 + 1);
+ memcpy((*s) + len1, str, len2 + 1);
}

void string_appendf(char **s, const char *format, ...)
@@ -461,18 +462,18 @@ void string_appendf(char **s, const char *format, ...)
return;
}

- *s = realloc(*s, len1 + len2 + 1);
- if (*s)
- memcpy((*s) + len1, s2, len2 + 1);
+ *s = xrealloc(*s, len1 + len2 + 1);
+ memcpy((*s) + len1, s2, len2 + 1);
free(s2);
}

void **parray_new(void)
{
- void **a = malloc(sizeof(*a));
+ void **a;
+
+ a = xmalloc(sizeof(*a));
+ *a = NULL;

- if (a)
- *a = NULL;
return a;
}

@@ -502,9 +503,7 @@ void parray_extend(void ***a, ...)
if (alloced < len + ilen) {
while (alloced < len + ilen)
alloced *= 2;
- *a = realloc(*a, alloced * sizeof **a);
- if (!*a)
- return;
+ *a = xrealloc(*a, alloced * sizeof **a);
}

va_start(ap, a);
diff --git a/util.h b/util.h
index 8c3aaff..cc521b6 100644
--- a/util.h
+++ b/util.h
@@ -292,11 +292,12 @@ void *xrealloc(void *ptr, size_t size);
char *xstrdup(const char *s);

/**
- * Get an allocated and formatted string. This is a wrapper around asprintf().
+ * Get an allocated and formatted string. This is a wrapper around asprintf()
+ * that terminates the process on errors.
*
* @param format printf() format string.
* @param ... printf() arguments.
- * @return Pointer to the allocated string, NULL on error.
+ * @return Pointer to the allocated string.
*/
#ifdef __GNUC__
__attribute__ ((format (printf, 1, 2)))
@@ -304,9 +305,10 @@ __attribute__ ((format (printf, 1, 2)))
char *string_newf(const char *format, ...);

/**
- * Reallocate a string and append another string to it.
+ * Reallocate a string and append another string to it. The process is
+ * terminated when the allocation fails.
*
- * @param s String that should be extended, set to NULL on error.
+ * @param s String that should be extended.
* @param str String appended to s.
*/
void string_append(char **s, const char *str);
@@ -314,26 +316,29 @@ void string_append(char **s, const char *str);
__attribute__ ((format (printf, 2, 3)))
#endif
/**
- * Reallocate a string and append a formatted string to it.
+ * Reallocate a string and append a formatted string to it. The process is
+ * terminated when the allocation fails.
*
- * @param s String that should be extended, set to NULL on error.
+ * @param s String that should be extended.
* @param format printf() format string.
* @param ... printf() arguments.
*/
void string_appendf(char **s, const char *format, ...);

/**
- * Get an empty array of pointers terminated by NULL.
+ * Get an empty array of pointers terminated by NULL. The process is terminated
+ * when the allocation fails.
*
- * @return Pointer to the allocated array, NULL on error.
+ * @return Pointer to the allocated array.
*/
void **parray_new(void);

/**
* Append pointer to a NULL-terminated pointer array. The array is reallocated
- * in exponentially increasing sizes.
+ * in exponentially increasing sizes. The process is terminated when the
+ * allocation fails.
*
- * @param a Pointer to pointer array, set to NULL on error.
+ * @param a Pointer to pointer array.
* @param p Pointer appended to the array.
*/
void parray_append(void ***a, void *p);
@@ -341,9 +346,10 @@ void parray_append(void ***a, void *p);

/**
* Append pointers to a NULL-terminated pointer array. The array is reallocated
- * in exponentially increasing sizes.
+ * in exponentially increasing sizes. The process is terminated when the
+ * allocation fails.
*
- * @param a Pointer to pointer array, set to NULL on error.
+ * @param a Pointer to pointer array.
* @param ... NULL-terminated list of pointers.
*/
void parray_extend(void ***a, ...);

[Miroslav Lichvar]

Add wrappers for malloc(), calloc(), realloc() and strdup() that check
if the allocation failed, print an error message and call exit(1). This
allows the caller to use the returned value without checking for errors.
---
util.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
util.h | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 89 insertions(+)

diff --git a/util.c b/util.c
index 9202c55..284657b 100644
--- a/util.c
+++ b/util.c
@@ -368,6 +368,58 @@ int is_running(void)
return running;
}

+void *xmalloc(size_t size)
+{
+ void *r;
+
+ r = malloc(size);
+ if (!r) {
+ pr_err("failed to allocate memory");
+ exit(1);
+ }
+
+ return r;
+}
+
+void *xcalloc(size_t nmemb, size_t size)
+{
+ void *r;
+
+ r = calloc(nmemb, size);
+ if (!r) {
+ pr_err("failed to allocate memory");
+ exit(1);
+ }
+
+ return r;
+}
+
+void *xrealloc(void *ptr, size_t size)
+{
+ void *r;
+
+ r = realloc(ptr, size);
+ if (!r) {
+ pr_err("failed to allocate memory");
+ exit(1);
+ }
+
+ return r;
+}
+
+char *xstrdup(const char *s)
+{
+ void *r;
+
+ r = strdup(s);
+ if (!r) {
+ pr_err("failed to allocate memory");
+ exit(1);
+ }
+
+ return r;
+}
+
char *string_newf(const char *format, ...)
{
va_list ap;
diff --git a/util.h b/util.h
index 758ee5f..8c3aaff 100644
--- a/util.h
+++ b/util.h
@@ -255,6 +255,43 @@ int handle_term_signals(void);
int is_running(void);

/**
+ * Allocate memory. This is a malloc() wrapper that terminates the process when
+ * the allocation fails.
+ *
+ * @param size Size of the block. Must be larger than 0.
+ * @return Pointer to the allocated memory.
+ */
+void *xmalloc(size_t size);
+
+/**
+ * Allocate and clear an array. This is a calloc() wrapper that terminates the
+ * process when the allocation fails.
+ *
+ * @param nmemb Number of elements. Must be larger than 0.
+ * @param size Size of the element. Must be larger than 0.
+ * @return Pointer to the allocated memory.
+ */
+void *xcalloc(size_t nmemb, size_t size);
+
+/**
+ * Reallocate memory. This is a realloc() wrapper that terminates the process
+ * when the allocation fails.
+ *
+ * @param size Size of the block. Must be larger than 0.
+ * @return Pointer to the allocated memory.
+ */
+void *xrealloc(void *ptr, size_t size);
+
+/**
+ * Duplicate a string. This is a strdup() wrapper that terminates the process
+ * when the allocation fails.
+ *
+ * @param s String that should be duplicated.
+ * @return Pointer to the duplicated string.
+ */
+char *xstrdup(const char *s);
+
+/**
* Get an allocated and formatted string. This is a wrapper around asprintf().
*
* @param format printf() format string.

[Miroslav Lichvar]

---
timemaster.c | 82 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/timemaster.c b/timemaster.c
index 76e48b3..9c71b42 100644
--- a/timemaster.c
+++ b/timemaster.c
@@ -131,7 +131,7 @@ static void extend_string_array(char ***a, char **strings)
char **s;

for (s = strings; *s; s++)
- parray_append((void ***)a, strdup(*s));
+ parray_append((void ***)a, xstrdup(*s));
}

static void extend_config_string(char **s, char **lines)
@@ -184,7 +184,7 @@ static void parse_words(char *s, char ***a)
while (*s) {
w = s;
s = parse_word(s);
- parray_append((void ***)a, strdup(w));
+ parray_append((void ***)a, xstrdup(w));
}
}

@@ -192,7 +192,7 @@ static void replace_string(char *s, char **str)
{
if (*str)
free(*str);
- *str = strdup(s);
+ *str = xstrdup(s);
}

static char *parse_section_name(char *s)
@@ -204,7 +204,7 @@ static char *parse_section_name(char *s)
;
*s2 = '\0';

- return strdup(s1);
+ return xstrdup(s1);
}

static void parse_setting(char *s, char **name, char **value)
@@ -267,10 +267,10 @@ static struct source *source_ntp_parse(char *parameter, char **settings)
}
}

- source = malloc(sizeof(*source));
+ source = xmalloc(sizeof(*source));
source->type = NTP_SERVER;
source->ntp = ntp_server;
- source->ntp.address = strdup(source->ntp.address);
+ source->ntp.address = xstrdup(source->ntp.address);

return source;
}
@@ -281,7 +281,7 @@ static struct source *source_ptp_parse(char *parameter, char **settings)
struct source *source;
int r = 0;

- source = malloc(sizeof(*source));
+ source = xmalloc(sizeof(*source));
source->type = PTP_DOMAIN;
source->ptp.delay = DEFAULT_PTP_DELAY;
source->ptp.ntp_poll = DEFAULT_PTP_NTP_POLL;
@@ -304,7 +304,7 @@ static struct source *source_ptp_parse(char *parameter, char **settings)
r = parse_int(value, &source->ptp.phc2sys_poll);
} else if (!strcasecmp(name, "ptp4l_option")) {
parray_append((void ***)&source->ptp.ptp4l_settings,
- strdup(value));
+ xstrdup(value));
} else if (!strcasecmp(name, "interfaces")) {
parse_words(value, &source->ptp.interfaces);
} else {
@@ -436,7 +436,7 @@ static void init_program_config(struct program_config *config,
const char *s;
va_list ap;

- config->path = strdup(name);
+ config->path = xstrdup(name);
config->settings = (char **)parray_new();
config->options = (char **)parray_new();

@@ -444,9 +444,9 @@ static void init_program_config(struct program_config *config,

/* add default options and settings */
while ((s = va_arg(ap, const char *)))
- parray_append((void ***)&config->options, strdup(s));
+ parray_append((void ***)&config->options, xstrdup(s));
while ((s = va_arg(ap, const char *)))
- parray_append((void ***)&config->settings, strdup(s));
+ parray_append((void ***)&config->settings, xstrdup(s));

va_end(ap);
}
@@ -477,7 +477,7 @@ static void config_destroy(struct timemaster_config *config)

static struct timemaster_config *config_parse(char *path)
{
- struct timemaster_config *config = calloc(1, sizeof(*config));
+ struct timemaster_config *config = xcalloc(1, sizeof(*config));
FILE *f;
char buf[4096], *line, *section_name = NULL;
char **section_lines = NULL;
@@ -485,7 +485,7 @@ static struct timemaster_config *config_parse(char *path)

config->sources = (struct source **)parray_new();
config->ntp_program = DEFAULT_NTP_PROGRAM;
- config->rundir = strdup(DEFAULT_RUNDIR);
+ config->rundir = xstrdup(DEFAULT_RUNDIR);

init_program_config(&config->chronyd, "chronyd",
NULL, DEFAULT_CHRONYD_SETTINGS, NULL);
@@ -536,7 +536,7 @@ static struct timemaster_config *config_parse(char *path)
break;
}

- parray_append((void ***)&section_lines, strdup(line));
+ parray_append((void ***)&section_lines, xstrdup(line));
}

if (!ret && section_name &&
@@ -565,15 +565,15 @@ static char **get_ptp4l_command(struct program_config *config,
{
char **command = (char **)parray_new();

- parray_append((void ***)&command, strdup(config->path));
+ parray_append((void ***)&command, xstrdup(config->path));
extend_string_array(&command, config->options);
parray_extend((void ***)&command,
- strdup("-f"), strdup(file->path),
- strdup(hw_ts ? "-H" : "-S"), NULL);
+ xstrdup("-f"), xstrdup(file->path),
+ xstrdup(hw_ts ? "-H" : "-S"), NULL);

for (; *interfaces; interfaces++)
parray_extend((void ***)&command,
- strdup("-i"), strdup(*interfaces), NULL);
+ xstrdup("-i"), xstrdup(*interfaces), NULL);

return command;
}
@@ -583,16 +583,16 @@ static char **get_phc2sys_command(struct program_config *config, int domain,
{
char **command = (char **)parray_new();

- parray_append((void ***)&command, strdup(config->path));
+ parray_append((void ***)&command, xstrdup(config->path));
extend_string_array(&command, config->options);
parray_extend((void ***)&command,
- strdup("-a"), strdup("-r"),
- strdup("-R"), string_newf("%.2f", poll > 0 ?
+ xstrdup("-a"), xstrdup("-r"),
+ xstrdup("-R"), string_newf("%.2f", poll > 0 ?
1.0 / (1 << poll) : 1 << -poll),
- strdup("-z"), strdup(uds_path),
- strdup("-n"), string_newf("%d", domain),
- strdup("-E"), strdup("ntpshm"),
- strdup("-M"), string_newf("%d", shm_segment), NULL);
+ xstrdup("-z"), xstrdup(uds_path),
+ xstrdup("-n"), string_newf("%d", domain),
+ xstrdup("-E"), xstrdup("ntpshm"),
+ xstrdup("-M"), string_newf("%d", shm_segment), NULL);

return command;
}
@@ -666,7 +666,7 @@ static int add_ptp_source(struct ptp_domain *source,
return 0;

/* get PHCs used by specified interfaces */
- phcs = malloc(num_interfaces * sizeof(int));
+ phcs = xmalloc(num_interfaces * sizeof(int));
for (i = 0; i < num_interfaces; i++) {
phcs[i] = -1;

@@ -719,7 +719,7 @@ static int add_ptp_source(struct ptp_domain *source,
}

/* don't use this PHC in other sources */
- phc = malloc(sizeof(int));
+ phc = xmalloc(sizeof(int));
*phc = phcs[i];
parray_append((void ***)allocated_phcs, phc);
}
@@ -727,10 +727,10 @@ static int add_ptp_source(struct ptp_domain *source,
uds_path = string_newf("%s/ptp4l.%d.socket",
config->rundir, *shm_segment);

- config_file = malloc(sizeof(*config_file));
+ config_file = xmalloc(sizeof(*config_file));
config_file->path = string_newf("%s/ptp4l.%d.conf",
config->rundir, *shm_segment);
- config_file->content = strdup("[global]\n");
+ config_file->content = xstrdup("[global]\n");
extend_config_string(&config_file->content,
config->ptp4l.settings);
extend_config_string(&config_file->content,
@@ -785,10 +785,10 @@ static char **get_chronyd_command(struct program_config *config,
{
char **command = (char **)parray_new();

- parray_append((void ***)&command, strdup(config->path));
+ parray_append((void ***)&command, xstrdup(config->path));
extend_string_array(&command, config->options);
- parray_extend((void ***)&command, strdup("-n"),
- strdup("-f"), strdup(file->path), NULL);
+ parray_extend((void ***)&command, xstrdup("-n"),
+ xstrdup("-f"), xstrdup(file->path), NULL);

return command;
}
@@ -798,10 +798,10 @@ static char **get_ntpd_command(struct program_config *config,
{
char **command = (char **)parray_new();

- parray_append((void ***)&command, strdup(config->path));
+ parray_append((void ***)&command, xstrdup(config->path));
extend_string_array(&command, config->options);
- parray_extend((void ***)&command, strdup("-n"),
- strdup("-c"), strdup(file->path), NULL);
+ parray_extend((void ***)&command, xstrdup("-n"),
+ xstrdup("-c"), xstrdup(file->path), NULL);

return command;
}
@@ -809,10 +809,10 @@ static char **get_ntpd_command(struct program_config *config,
static struct config_file *add_ntp_program(struct timemaster_config *config,
struct script *script)
{
- struct config_file *ntp_config = malloc(sizeof(*ntp_config));
+ struct config_file *ntp_config = xmalloc(sizeof(*ntp_config));
char **command = NULL;

- ntp_config->content = strdup("");
+ ntp_config->content = xstrdup("");

switch (config->ntp_program) {
case CHRONYD:
@@ -861,7 +861,7 @@ static void script_destroy(struct script *script)

static struct script *script_create(struct timemaster_config *config)
{
- struct script *script = malloc(sizeof(*script));
+ struct script *script = xmalloc(sizeof(*script));
struct source *source, **sources;
struct config_file *ntp_config = NULL;
int **allocated_phcs = (int **)parray_new();
@@ -942,7 +942,7 @@ static pid_t start_program(char **command, sigset_t *mask)
}
#endif

- for (s = strdup(""), arg = command; *arg; arg++)
+ for (s = xstrdup(""), arg = command; *arg; arg++)
string_appendf(&s, "%s ", *arg);

pr_info("process %d started: %s", pid, s);
@@ -960,7 +960,7 @@ static int create_config_files(struct config_file **configs)
struct stat st;

for (; (config = *configs); configs++) {
- tmp = strdup(config->path);
+ tmp = xstrdup(config->path);
dir = dirname(tmp);
if (stat(dir, &st) < 0 && errno == ENOENT &&
mkdir(dir, 0755) < 0) {
@@ -1030,7 +1030,7 @@ static int script_run(struct script *script)
for (num_commands = 0; script->commands[num_commands]; num_commands++)
;

- pids = calloc(num_commands, sizeof(*pids));
+ pids = xcalloc(num_commands, sizeof(*pids));

for (i = 0; i < num_commands; i++) {
pids[i] = start_program(script->commands[i], &old_mask);

[Richard Cochran]

Post by Miroslav Lichvar
As per the discussion on linuxptp-users, this is a set that tries to
fix the unhandled errors in memory allocation in timemaster.

Series applied.

Thanks,
Richard

------------------------------------------------------------------------------