Skip to content

Commit

Permalink
Add more error handling for memory allocations (Issue #277)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michaelrsweet
michaelrsweet committed Oct 26, 2021
1 parent e058082 commit 759e4db
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changes in Mini-XML 3.2.1

- Cleaned up usage of `free` throughout the library (Issue #276)
- Added more error handling to the library (Issue #277)
- Fixed potential memory leak in `mxmlLoad*` functions (Issue #278, Issue #279)
- Fixed `mxmlSaveString` with a buffer size of 0 (Issue #284)
- Fixed `MXML_MINOR_VERSION` value in "mxml.h" (Issue #285)
Expand Down
14 changes: 10 additions & 4 deletions mxml-attr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ mxmlElementSetAttr(mxml_node_t *node, /* I - Element node */
return;

if (value)
valuec = strdup(value);
{
if ((valuec = strdup(value)) == NULL)
{
mxml_error("Unable to allocate memory for attribute '%s' in element %s.", name, node->value.element.name);
return;
}
}
else
valuec = NULL;

Expand Down Expand Up @@ -269,7 +275,7 @@ mxmlElementSetAttrf(mxml_node_t *node, /* I - Element node */
va_end(ap);

if (!value)
mxml_error("Unable to allocate memory for attribute '%s' in element %s!",
mxml_error("Unable to allocate memory for attribute '%s' in element %s.",
name, node->value.element.name);
else if (mxml_set_attr(node, name, value))
free(value);
Expand Down Expand Up @@ -320,7 +326,7 @@ mxml_set_attr(mxml_node_t *node, /* I - Element node */

if (!attr)
{
mxml_error("Unable to allocate memory for attribute '%s' in element %s!",
mxml_error("Unable to allocate memory for attribute '%s' in element %s.",
name, node->value.element.name);
return (-1);
}
Expand All @@ -330,7 +336,7 @@ mxml_set_attr(mxml_node_t *node, /* I - Element node */

if ((attr->name = strdup(name)) == NULL)
{
mxml_error("Unable to allocate memory for attribute '%s' in element %s!",
mxml_error("Unable to allocate memory for attribute '%s' in element %s.",
name, node->value.element.name);
return (-1);
}
Expand Down
14 changes: 10 additions & 4 deletions mxml-index.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,19 @@ mxmlIndexNew(mxml_node_t *node, /* I - XML node tree */

if ((ind = calloc(1, sizeof(mxml_index_t))) == NULL)
{
mxml_error("Unable to allocate %d bytes for index - %s", (int)sizeof(mxml_index_t), strerror(errno));
mxml_error("Unable to allocate memory for index.");
return (NULL);
}

if (attr)
ind->attr = strdup(attr);
{
if ((ind->attr = strdup(attr)) == NULL)
{
mxml_error("Unable to allocate memory for index attribute.");
free(ind);
return (NULL);
}
}

if (!element && !attr)
current = node;
Expand All @@ -353,8 +360,7 @@ mxmlIndexNew(mxml_node_t *node, /* I - XML node tree */
* Unable to allocate memory for the index, so abort...
*/

mxml_error("Unable to allocate %d bytes for index: %s", (int)((ind->alloc_nodes + 64) * sizeof(mxml_node_t *)), strerror(errno));

mxml_error("Unable to allocate memory for index nodes.");
mxmlIndexDelete(ind);
return (NULL);
}
Expand Down
131 changes: 121 additions & 10 deletions mxml-set.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ int /* O - 0 on success, -1 on failure */
mxmlSetCDATA(mxml_node_t *node, /* I - Node to set */
const char *data) /* I - New data string */
{
char *s; /* New element name */


/*
* Range check input...
*/
Expand All @@ -39,19 +42,39 @@ mxmlSetCDATA(mxml_node_t *node, /* I - Node to set */
!strncmp(node->child->value.element.name, "![CDATA[", 8))
node = node->child;

if (!node || node->type != MXML_ELEMENT || !data ||
if (!node || node->type != MXML_ELEMENT ||
strncmp(node->value.element.name, "![CDATA[", 8))
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!data)
{
mxml_error("NULL string not allowed.");
return (-1);
}

if (data == (node->value.element.name + 8))
{
/*
* Don't change the value...
*/

return (0);
}

/*
* Allocate the new value, free any old element value, and set the new value...
*/

if ((s = _mxml_strdupf("![CDATA[%s", data)) == NULL)
{
mxml_error("Unable to allocate memory for CDATA.");
return (-1);
}

free(node->value.element.name);
node->value.element.name = _mxml_strdupf("![CDATA[%s", data);
node->value.element.name = s;

return (0);
}
Expand Down Expand Up @@ -80,7 +103,10 @@ mxmlSetCustom(
node = node->child;

if (!node || node->type != MXML_CUSTOM)
{
mxml_error("Wrong node type.");
return (-1);
}

if (data == node->value.custom.data)
{
Expand Down Expand Up @@ -112,12 +138,23 @@ int /* O - 0 on success, -1 on failure */
mxmlSetElement(mxml_node_t *node, /* I - Node to set */
const char *name) /* I - New name string */
{
char *s; /* New name string */


/*
* Range check input...
*/

if (!node || node->type != MXML_ELEMENT || !name)
if (!node || node->type != MXML_ELEMENT)
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!name)
{
mxml_error("NULL string not allowed.");
return (-1);
}

if (name == node->value.element.name)
return (0);
Expand All @@ -126,8 +163,14 @@ mxmlSetElement(mxml_node_t *node, /* I - Node to set */
* Free any old element value and set the new value...
*/

if ((s = strdup(name)) == NULL)
{
mxml_error("Unable to allocate memory for element name.");
return (-1);
}

free(node->value.element.name);
node->value.element.name = strdup(name);
node->value.element.name = s;

return (0);
}
Expand All @@ -152,7 +195,10 @@ mxmlSetInteger(mxml_node_t *node, /* I - Node to set */
node = node->child;

if (!node || node->type != MXML_INTEGER)
{
mxml_error("Wrong node type.");
return (-1);
}

/*
* Set the new value and return...
Expand All @@ -174,6 +220,9 @@ int /* O - 0 on success, -1 on failure */
mxmlSetOpaque(mxml_node_t *node, /* I - Node to set */
const char *opaque) /* I - Opaque string */
{
char *s; /* New opaque string */


/*
* Range check input...
*/
Expand All @@ -182,8 +231,16 @@ mxmlSetOpaque(mxml_node_t *node, /* I - Node to set */
node->child && node->child->type == MXML_OPAQUE)
node = node->child;

if (!node || node->type != MXML_OPAQUE || !opaque)
if (!node || node->type != MXML_OPAQUE)
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!opaque)
{
mxml_error("NULL string not allowed.");
return (-1);
}

if (node->value.opaque == opaque)
return (0);
Expand All @@ -192,8 +249,14 @@ mxmlSetOpaque(mxml_node_t *node, /* I - Node to set */
* Free any old opaque value and set the new value...
*/

if ((s = strdup(opaque)) == NULL)
{
mxml_error("Unable to allocate memory for opaque string.");
return (-1);
}

free(node->value.opaque);
node->value.opaque = strdup(opaque);
node->value.opaque = s;

return (0);
}
Expand Down Expand Up @@ -224,8 +287,16 @@ mxmlSetOpaquef(mxml_node_t *node, /* I - Node to set */
node->child && node->child->type == MXML_OPAQUE)
node = node->child;

if (!node || node->type != MXML_OPAQUE || !format)
if (!node || node->type != MXML_OPAQUE)
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!format)
{
mxml_error("NULL string not allowed.");
return (-1);
}

/*
* Format the new string, free any old string value, and set the new value...
Expand All @@ -235,6 +306,12 @@ mxmlSetOpaquef(mxml_node_t *node, /* I - Node to set */
s = _mxml_vstrdupf(format, ap);
va_end(ap);

if (!s)
{
mxml_error("Unable to allocate memory for opaque string.");
return (-1);
}

free(node->value.opaque);
node->value.opaque = s;

Expand All @@ -261,7 +338,10 @@ mxmlSetReal(mxml_node_t *node, /* I - Node to set */
node = node->child;

if (!node || node->type != MXML_REAL)
{
mxml_error("Wrong node type.");
return (-1);
}

/*
* Set the new value and return...
Expand All @@ -284,6 +364,9 @@ mxmlSetText(mxml_node_t *node, /* I - Node to set */
int whitespace, /* I - 1 = leading whitespace, 0 = no whitespace */
const char *string) /* I - String */
{
char *s; /* New string */


/*
* Range check input...
*/
Expand All @@ -292,8 +375,16 @@ mxmlSetText(mxml_node_t *node, /* I - Node to set */
node->child && node->child->type == MXML_TEXT)
node = node->child;

if (!node || node->type != MXML_TEXT || !string)
if (!node || node->type != MXML_TEXT)
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!string)
{
mxml_error("NULL string not allowed.");
return (-1);
}

if (string == node->value.text.string)
{
Expand All @@ -305,10 +396,16 @@ mxmlSetText(mxml_node_t *node, /* I - Node to set */
* Free any old string value and set the new value...
*/

if ((s = strdup(string)) == NULL)
{
mxml_error("Unable to allocate memory for text string.");
return (-1);
}

free(node->value.text.string);

node->value.text.whitespace = whitespace;
node->value.text.string = strdup(string);
node->value.text.string = s;

return (0);
}
Expand Down Expand Up @@ -338,8 +435,16 @@ mxmlSetTextf(mxml_node_t *node, /* I - Node to set */
node->child && node->child->type == MXML_TEXT)
node = node->child;

if (!node || node->type != MXML_TEXT || !format)
if (!node || node->type != MXML_TEXT)
{
mxml_error("Wrong node type.");
return (-1);
}
else if (!format)
{
mxml_error("NULL string not allowed.");
return (-1);
}

/*
* Free any old string value and set the new value...
Expand All @@ -349,6 +454,12 @@ mxmlSetTextf(mxml_node_t *node, /* I - Node to set */
s = _mxml_vstrdupf(format, ap);
va_end(ap);

if (!s)
{
mxml_error("Unable to allocate memory for text string.");
return (-1);
}

free(node->value.text.string);

node->value.text.whitespace = whitespace;
Expand Down

0 comments on commit 759e4db

Please sign in to comment.