os_xml review #142

cgzones · 2014-03-15T10:39:09Z

changes:

remove global XML_VAR compile directive
restructure header structure (os_xml.h + os_xml_writer.h -> os_xml.h (for external includes) + os_xml_internal.h (for internal macros)
always ensure valid OS_XML state so OS_ClearXML() never encounter a nullpointer or memory leak
remove unused function _checkmemory()
clean up memory in failure branches
fix a bunch of compiler warnings
add test cases

…+ os_xml_internal.h

…educe os_xml.c ones to local scope

…ld) and use nonnull attributes for static analysers

formatting with astyle --indent=spaces=4 --indent-switches --lineend=linux --style=stroustrup --pad-oper --add-brackets --align-pointer=name os_xml/*.c astyle --indent=spaces=4 --indent-switches --lineend=linux --style=stroustrup --pad-oper --add-brackets --align-pointer=name os_xml/*.h see https://github.com/ossec/ossec-docs/blob/fae75e0beb6201d7edec5d65d30d657c565cfedc/docs/oRFC/orfc-2.rst

This reverts commit 1e94f63.

jrossi · 2014-03-15T11:48:25Z

I have been watching the branch as you were working on it. This is great. Thank you for all the work on this. I am running this on a few different OS this weekend to try it out. Hope you understand that lots of code changes and lots to review. Thank you doing TDD it make it much simpler :). I should be done reviewing sometime today.

cgzones · 2014-03-15T13:44:36Z

Of course i understand that it takes time to review.

I edited the test case source file a bit so it is applicable on the unchanged source:

/* Copyright (C) 2014 Trend Micro Inc.
 * All rights reserved.
 *
 * This program is a free software; you can redistribute it
 * and/or modify it under the terms of the GNU General Public
 * License (version 2) as published by the FSF - Free Software
 * Foundation
 */

#include <check.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

#include "../os_xml/os_xml.h"
#include "../os_xml/os_xml_writer.h"

Suite *test_suite(void);

static void create_xml_file(const char *str, char file_name[], size_t length)
{
    strncpy(file_name, "/tmp/tmp_file-XXXXXX", length);
    int fd = mkstemp(file_name);

    write(fd, str, strlen(str));
    close(fd);
}

static void nodecat(XML_NODE node, OS_XML *xml, char *buffer) {
    int i = 0;
    /* write node */
    while(node[i])
    {
        strncat(buffer, "<", 1);
        ck_assert_ptr_ne(node[i]->element, NULL);
        strncat(buffer, node[i]->element, strlen(node[i]->element));
        /* write attributes */
        if(node[i]->attributes)
        {
            ck_assert_ptr_ne(node[i]->values, NULL);
            int j = 0;
            while(node[i]->attributes[j])
            {
                strncat(buffer, " ", 1);
                ck_assert_ptr_ne(node[i]->values[j], NULL);
                strncat(buffer, node[i]->attributes[j], strlen(node[i]->attributes[j]));
                strncat(buffer, "=", 1);
                strncat(buffer, "\"", 1);
                strncat(buffer, node[i]->values[j], strlen(node[i]->values[j]));
                strncat(buffer, "\"", 1);
                j++;
            }
            ck_assert_ptr_eq(node[i]->values[j], NULL);
        }
        else
        {
            ck_assert_ptr_eq(node[i]->values, NULL);
        }
        strncat(buffer, ">", 1);
        ck_assert_ptr_ne(node[i]->content, NULL);
        strncat(buffer, node[i]->content, strlen(node[i]->content));

        /* write children */
        XML_NODE child = OS_GetElementsbyNode(xml, node[i]);
        if(child != NULL)
        {
            nodecat(child, xml, buffer);
            OS_ClearNode(child);
        }

        /* close node */
        strncat(buffer, "</", 2);
        strncat(buffer, node[i]->element, strlen(node[i]->element));
        strncat(buffer, ">", 1);
        i++;
    }
}

static void assert_os_xml_eq_str(OS_XML *xml, const char *xml_str)
{
    XML_NODE node = OS_GetElementsbyNode(xml, NULL);
    if(node == NULL)
    {
        ck_assert_str_eq(xml_str, "");
        return;
    }

    char *buffer = (char *) malloc(6144 * sizeof(char));
    buffer[0] = '\0';
    nodecat(node, xml, buffer);
    OS_ClearNode(node);

    ck_assert_str_eq(buffer, xml_str);

    free(buffer);
}

static void assert_os_xml_eq(const char *parse_str, const char *xml_str)
{
    char xml_file_name[256];
    create_xml_file(parse_str, xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    assert_os_xml_eq_str(&xml, xml_str);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}

START_TEST(test_simplenodes)
{
    assert_os_xml_eq("", "");
    assert_os_xml_eq("<root></root>", "<root></root>");
    assert_os_xml_eq("<root/>", "<root></root>");
}
END_TEST


START_TEST(test_multiplenodes)
{
    assert_os_xml_eq(
            "<root1></root1>"
            "<root2></root2>"
            "<root3/>",
            "<root1></root1>"
            "<root2></root2>"
            "<root3></root3>");
}
END_TEST

START_TEST(test_children)
{
    assert_os_xml_eq(
            "<root1>"
            "<child1></child1>"
            "</root1>"
            "<root2>"
            "<child1></child1>"
            "<child2/>"
            "<child3>"
            "<child3.1></child3.1>"
            "</child3>"
            "</root2>"
            "<root3></root3>",
            "<root1>"
            "<child1></child1>"
            "</root1>"
            "<root2>"
            "<child1></child1>"
            "<child2></child2>"
            "<child3>"
            "<child3.1></child3.1>"
            "</child3>"
            "</root2>"
            "<root3></root3>");
}
END_TEST


START_TEST(test_attributes)
{
    assert_os_xml_eq(
            "<root attr1=\"test\" attr2=\"1\"></root>",
            "<root attr1=\"test\" attr2=\"1\"></root>");
}
END_TEST

START_TEST(test_variables)
{
    assert_os_xml_eq(
            "<var name=\"var1\">value1</var>"
            "<var name=\"var2\">value2</var>"
            "<root attr1=\"$var1\" attr2=\"1\">$var2</root>"
            "<root attr1=\"blah$var1\" attr2=\"1\">blah$var2</root>"
            "<root attr1=\"blah$var1$var2 blah\" attr2=\"1\">blah$var2$var1 blah</root>",
            "<root attr1=\"value1\" attr2=\"1\">value2</root>"
            "<root attr1=\"blahvalue1\" attr2=\"1\">blahvalue2</root>"
            "<root attr1=\"blahvalue1value2 blah\" attr2=\"1\">blahvalue2value1 blah</root>");
}
END_TEST

START_TEST(test_comments)
{
    assert_os_xml_eq(
            "<root1/><!comment!><root2/>",
            "<root1></root1><root2></root2>");
    assert_os_xml_eq(
            "<root1/><!--comment--><root2/>",
            "<root1></root1><root2></root2>");
}
END_TEST

START_TEST(test_specialchars)
{
    assert_os_xml_eq(
            "<var name=\"var1\">value1</var>"
            "<root1>\\</root1\\></root1>",
            "<root1>\\</root1\\></root1>");
}
END_TEST

START_TEST(test_linecounter)
{
    char xml_file_name[256];
    create_xml_file("<root1/>\n<root2/>\n<root3/>" , xml_file_name, 256);
    OS_XML xml;
    XML_NODE node;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    ck_assert_ptr_ne(node = OS_GetElementsbyNode(&xml, NULL), NULL);
    ck_assert_int_eq(xml.ln[0], 1);
    ck_assert_int_eq(xml.ln[1], 2);
    ck_assert_int_eq(xml.ln[2], 3);

    OS_ClearNode(node);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_invalidfile)
{
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML("invalid_file.inv", &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: File 'invalid_file.inv' not found.");
    ck_assert_int_eq(xml.err_line, 0);

    //OS_ClearXML(&xml);
}
END_TEST

START_TEST(test_unclosednode)
{
    char xml_file_name[256];
    create_xml_file("<root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XML ERR: End of file and some elements were not closed");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_unclosedcomment)
{
    char xml_file_name[256];
    create_xml_file("<!-- comment", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XML ERR: Comment not closed. Bad XML.");
    ck_assert_int_eq(xml.err_line, 1);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_nodenotopened)
{
    char xml_file_name[256];
    create_xml_file("</root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XML ERR: Bad formed XML. Element not opened");
    ck_assert_int_eq(xml.err_line, 1);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_unclosedattribute)
{
    char xml_file_name[256];
    create_xml_file("<root attr=\"attribute></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: Attribute 'attr' not closed.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_unquotedattribute)
{
    char xml_file_name[256];
    create_xml_file("<root attr=attribute></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: Attribute 'attr' not followed by a \" or '.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_infiniteattribute)
{
    char xml_file_name[256];
    create_xml_file("<root attr", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: End of file while reading an attribute.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_invalidvariablename)
{
    char xml_file_name[256];
    create_xml_file("<var test=\"test\"></var>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_ne(OS_ApplyVariables(&xml), 0);
    ck_assert_str_eq(xml.err, "XML_ERR: Only \"name\" is allowed as an attribute for a variable");
    ck_assert_int_eq(xml.err_line, 0);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_unknownvariable)
{
    char xml_file_name[256];
    create_xml_file("<var name=\"test\">content</var><root>$var</root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_ne(OS_ApplyVariables(&xml), 0);
    ck_assert_str_eq(xml.err, "XML_ERR: Unknown variable: var");
    ck_assert_int_eq(xml.err_line, 0);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_infiniteattribute2)
{
    char xml_file_name[256];
    create_xml_file("<root attr=", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: Attribute 'attr' not followed by a \" or '.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_invalidattributeclosing)
{
    char xml_file_name[256];
    create_xml_file("<root attr=''", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: Bad attribute closing for 'attr'=''.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_infiniteattribute3)
{
    char xml_file_name[256];
    create_xml_file("<root attr='", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_ne(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_str_eq(xml.err, "XMLERR: End of file while reading an attribute.");
    ck_assert_int_eq(xml.err_line, 1);

    //OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

//TODO
/*START_TEST(test_unknownvariable)
{
    char xml_file_name[256];
    create_xml_file("<root>$var</root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_ne(OS_ApplyVariables(&xml), 0);
    ck_assert_str_eq(xml.err, "XML_ERR: Unknown variable: var");
    ck_assert_int_eq(xml.err_line, 0);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST*/

START_TEST(test_oselementsexists)
{
    char xml_file_name[256];
    create_xml_file("<root></root><root1/><root/>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    ck_assert_int_eq(OS_RootElementExist(&xml, "root"), 2);
    ck_assert_int_eq(OS_RootElementExist(&xml, "root1"), 1);
    ck_assert_int_eq(OS_RootElementExist(&xml, "root2"), 0);
    //ck_assert_int_eq(OS_RootElementExist(&xml, NULL), 0);
    ck_assert_int_eq(OS_ElementExist(&xml, NULL), 0);

    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osgetonecontentforelement)
{
    char xml_file_name[256];
    create_xml_file("<root><child>test</child><child>test2</child><child2>test</child2></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path1[] = { "root", "child", NULL };
    const char *xml_path2[] = { "root", "child2", NULL };
    const char *xml_path3[] = { "root", "child3", NULL };
    char *content1, *content2;
    ck_assert_str_eq(content1 = OS_GetOneContentforElement(&xml, xml_path1), "test");
    ck_assert_str_eq(content2 = OS_GetOneContentforElement(&xml, xml_path2), "test");
    ck_assert_ptr_eq(OS_GetOneContentforElement(&xml, xml_path3), NULL);

    free(content1);
    free(content2);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

static void assert_ox_xml_write_eq(const char *xml_str_old, const char *xml_str_new,const char **xml_path, const char *oldval, const char *newval)
{
    char xml_in_file_name[256];
    create_xml_file(xml_str_old, xml_in_file_name, 256);
    char xml_out_file_name[256];
    create_xml_file("", xml_out_file_name, 256);

    ck_assert_int_eq(OS_WriteXML(xml_in_file_name, xml_out_file_name, xml_path, NULL, oldval, newval, 0), 0);

    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_out_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    assert_os_xml_eq_str(&xml, xml_str_new);

    OS_ClearXML(&xml);
    unlink(xml_in_file_name);
    unlink(xml_out_file_name);
}

START_TEST(test_oswritexml_success)
{
    const char *xml_path[] = { "root", "child", NULL };
    assert_ox_xml_write_eq(
            "<root><child>test</child></root>",
            "<root><child>test_new</child></root>",
            xml_path, "test", "test_new");

    assert_ox_xml_write_eq(
            "<root><child>test</child></root>",
            "<root><child>test</child></root>",
            xml_path, "test", "test");

    assert_ox_xml_write_eq(
            "<root><child></child></root>",
            "<root><child>test</child></root>",
            xml_path, "test", "test");

    assert_ox_xml_write_eq(
                "<root2><child></child></root2>",
                "<root2><child></child></root2><root>\n <child>test</child></root>",
                xml_path, NULL, "test");
}
END_TEST

START_TEST(test_oswritexml_failures)
{
    char xml_in_file_name[256];
    create_xml_file("<root><child>test</child></root>", xml_in_file_name, 256);
    char xml_out_file_name[256];
    create_xml_file("", xml_out_file_name, 256);
    const char *xml_path[] = { "root", "child", NULL };

    ck_assert_int_eq(OS_WriteXML(xml_in_file_name, xml_out_file_name, NULL, NULL, "test", "test_new", 0), XMLW_ERROR);
    ck_assert_int_eq(OS_WriteXML(xml_in_file_name, xml_out_file_name, xml_path, NULL, "test", NULL, 0), XMLW_ERROR);
    ck_assert_int_eq(OS_WriteXML("invalid", xml_out_file_name, xml_path, NULL, "test", "test_new", 0), XMLW_NOIN);
    ck_assert_int_eq(OS_WriteXML(xml_in_file_name, "??invalid<<!!\"\"//\\\\", xml_path, NULL, "test", "test_new", 0), XMLW_NOOUT);

    unlink(xml_in_file_name);
    unlink(xml_out_file_name);
}
END_TEST

START_TEST(test_osgetattributecontent)
{
    char xml_file_name[256];
    create_xml_file("<root attr=\"value\" attr2=\"value1\" attr2=\"value2\"></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path1[] = { "root", NULL };
    const char *xml_path2[] = { NULL };
    char *content1, *content2, *content3, *content4;
    ck_assert_str_eq(content1 = OS_GetAttributeContent(&xml, xml_path1, "attr"), "value");
    ck_assert_str_eq(content2 = OS_GetAttributeContent(&xml, xml_path1, "attr2"), "value1");
    ck_assert_str_eq(content3 = OS_GetAttributeContent(&xml, xml_path1, "attr3"), "");
    ck_assert_str_eq(content4 = OS_GetAttributeContent(&xml, xml_path1, NULL), "");
    ck_assert_ptr_eq(OS_GetAttributeContent(&xml, NULL, NULL), NULL);
    ck_assert_ptr_eq(OS_GetAttributeContent(&xml, xml_path2, NULL), NULL);


    free(content1);
    free(content2);
    free(content3);
    free(content4);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osgetcontents)
{
    char xml_file_name[256];
    create_xml_file("<root>value</root><root>value2</root><root2>value</root2>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path1[] = { "root", NULL };
    const char *xml_path2[] = { "root2", NULL };
    char **content1, **content2;
    ck_assert_ptr_ne(content1 = OS_GetContents(&xml, xml_path1), NULL);
    ck_assert_str_eq(content1[0], "value");
    ck_assert_str_eq(content1[1], "value2");
    ck_assert_ptr_eq(content1[2], NULL);

    ck_assert_ptr_ne(content2 = OS_GetContents(&xml, xml_path2), NULL);
    ck_assert_str_eq(content2[0], "value");
    ck_assert_ptr_eq(content2[1], NULL);

    ck_assert_ptr_eq(OS_GetContents(&xml, NULL), NULL);

    int i = 0;
    while(content1[i])
        free(content1[i++]);
    free(content1);

    i = 0;
    while(content2[i])
        free(content2[i++]);
    free(content2);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osgetelementcontent)
{
    char xml_file_name[256];
    create_xml_file("<root>value</root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path[] = { "root", NULL };
    char **content;
    ck_assert_ptr_ne(content = OS_GetElementContent(&xml, xml_path), NULL);
    ck_assert_str_eq(content[0], "value");
    ck_assert_ptr_eq(content[1], NULL);

    int i = 0;
    while(content[i])
        free(content[i++]);
    free(content);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osgetelements)
{
    char xml_file_name[256];
    create_xml_file("<root><child1/><child2/></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path[] = { "root", NULL };
    char **content1, **content2;
    ck_assert_ptr_ne(content1 = OS_GetElements(&xml, xml_path), NULL);
    ck_assert_str_eq(content1[0], "child1");
    ck_assert_str_eq(content1[1], "child2");
    ck_assert_ptr_eq(content1[2], NULL);
    ck_assert_ptr_ne(content2 = OS_GetElements(&xml, NULL), NULL);
    ck_assert_str_eq(content2[0], "root");
    ck_assert_ptr_eq(content2[1], NULL);
    const char *xml_path2[] = { NULL };
    ck_assert_ptr_eq(OS_GetElements(&xml,  xml_path2), NULL);

    int i = 0;
    while(content1[i])
        free(content1[i++]);
    free(content1);
    i = 0;
    while(content2[i])
        free(content2[i++]);
    free(content2);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osgetattributes)
{
    char xml_file_name[256];
    create_xml_file("<root attr1=\"1\" attr2=\"2\"></root>", xml_file_name, 256);
    OS_XML xml;
    ck_assert_int_eq(OS_ReadXML(xml_file_name, &xml), 0);
    ck_assert_int_eq(OS_ApplyVariables(&xml), 0);
    const char *xml_path[] = { "root", NULL };
    char **content;
    ck_assert_ptr_ne(content = OS_GetAttributes(&xml, xml_path), NULL);
    ck_assert_str_eq(content[0], "attr1");
    ck_assert_str_eq(content[1], "attr2");
    ck_assert_ptr_eq(content[2], NULL);

    int i = 0;
    while(content[i])
        free(content[i++]);
    free(content);
    OS_ClearXML(&xml);
    unlink(xml_file_name);
}
END_TEST

START_TEST(test_osclearnode)
{
    OS_ClearNode(NULL);
}
END_TEST

Suite *test_suite(void)
{
    Suite *s = suite_create("os_xml");

    /* Core test case */
    TCase *tc_core = tcase_create("Core");
    tcase_add_test(tc_core, test_simplenodes);
    tcase_add_test(tc_core, test_multiplenodes);
    tcase_add_test(tc_core, test_children);
    tcase_add_test(tc_core, test_attributes);
    tcase_add_test(tc_core, test_variables);
    tcase_add_test(tc_core, test_comments);
    tcase_add_test(tc_core, test_specialchars);
    tcase_add_test(tc_core, test_linecounter);
    tcase_add_test(tc_core, test_invalidfile);
    tcase_add_test(tc_core, test_unclosednode);
    tcase_add_test(tc_core, test_unclosedcomment);
    tcase_add_test(tc_core, test_nodenotopened);
    tcase_add_test(tc_core, test_unclosedattribute);
    tcase_add_test(tc_core, test_infiniteattribute);
    tcase_add_test(tc_core, test_unquotedattribute);
    tcase_add_test(tc_core, test_invalidvariablename);
    tcase_add_test(tc_core, test_unknownvariable);
    tcase_add_test(tc_core, test_infiniteattribute2);
    tcase_add_test(tc_core, test_invalidattributeclosing);
    tcase_add_test(tc_core, test_infiniteattribute3);
    tcase_add_test(tc_core, test_oselementsexists);
    tcase_add_test(tc_core, test_osgetonecontentforelement);
    tcase_add_test(tc_core, test_oswritexml_success);
    tcase_add_test(tc_core, test_oswritexml_failures);
    tcase_add_test(tc_core, test_osgetattributecontent);
    tcase_add_test(tc_core, test_osgetcontents);
    tcase_add_test(tc_core, test_osgetelementcontent);
    tcase_add_test(tc_core, test_osgetelements);
    tcase_add_test(tc_core, test_osgetattributes);
    tcase_add_test(tc_core, test_osclearnode);
    suite_add_tcase(s, tc_core);

    return (s);
}

int main(void)
{
    Suite *s = test_suite();
    SRunner *sr = srunner_create(s);
    srunner_run_all(sr, CK_NORMAL);
    int number_failed = srunner_ntests_failed(sr);
    srunner_free(sr);

    return ((number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE);
}

I included the old header os_writer_xml.h, added the unused arguments to os_write_xml() and commented out the crashing memory clean up calls

…when called

jrossi · 2014-03-18T22:18:52Z

Really sorry I have not got this pulled in yet. I have it running on a test setup and will merge tomorrow as thinks are looking good so far.

…ng xml node attributes

fixed #149

jrossi · 2014-03-20T17:12:34Z

I have been running this with ossec-vagrant for a bit and works fine for everything I hit it with. I have been working with os_xml for to long and know it's ways and work around them by habit.

Looks lik @loonhxl has so fixes pulled into this too.

Plus the code changes will help us to find other bugs.

os_xml review

cgzones added 30 commits March 14, 2014 10:02

add unit tests for os_xml

bd72dea

simplyfy os_xml testing code

f0f95ad

os_xml: more testcases

694afdf

travis: install lcov for testing

8ada165

os_xml: more testcases

8ddb478

[os_xml] update makefile

86cd523

[os_xml] do not define XML_VAR by global compiler flag

e3c1802

[os_xml] make local functions static

325c050

[os_xml] remove unused function checkmemory()

d892bf3

[os_xml] make usage of keyword const

697b386

[os_xml] fixing compiler warnings

88a849c

[os_xml] replac shared.h with standard headers

d23e3b0

[os_xml] remove dead code and unused macros

2ef9b7d

[os_xml] restructure headers: os_xml.h + os_xml_writer.h -> os_xml.h …

40b08f5

…+ os_xml_internal.h

[os_xml] remove unused global line variable from os_write_xml.c and r…

320f86b

…educe os_xml.c ones to local scope

[os_xml] fix some cppcheck warnings

eddd4eb

[os_xml] fix some conversion warnings

d8faeae

[os_xml] remove unused struct element

cedac3f

[os_xml] fix more conversion warnings

678160f

[os_xml] comment out dead code

5cc52db

[os_xml] always ensure a freeable xml structure state

c3bb8f7

[tests] fix memory leaks

a7591a4

[os_xml] fix memory leak

0634369

[os_xml] fix memory leaks on error paths

69e59c5

[os_xml] fix two possible nullpointer dereferences (found by scan-bui…

3974a1d

…ld) and use nonnull attributes for static analysers

[os_xml] fix -Wsign-conversion

5c7c863

revert back some files for smaller merge

eb7f3b7

[tests] build tests for os_xml after merge cleanup

1c95914

[tests] add some more tests

e383903

cgzones added 3 commits March 14, 2014 10:11

[tests] delete unused file

b147d7b

remove coverage leftovers

600c911

Revert "[os_xml] astyle format"

4642890

This reverts commit 1e94f63.

[os_xml] set some function parameters to nonnull and ensure they are …

f607e62

…when called

jrossi added bug labels Mar 17, 2014

jrossi self-assigned this Mar 17, 2014

jrossi added this to the ossec-hids-2.8 milestone Mar 17, 2014

cgzones added 3 commits March 18, 2014 12:17

[os_xml] catch all possible memory errors

4c81c8d

[os_xml] OS_ClearXML: null pointers

0bc2533

[os_xml] model length of xml error buffer as macro

6132088

cgzones and others added 11 commits March 19, 2014 11:34

[os_xml] fix #149 parcel

acb89fa

[os_xml] fix #149 '/' value character

0960051

[os_xml] alert on duplicate attribute

336554e

[os_xml] fix false positve duplicate attributes between different nodes

b33921a

[os_xml] duplicate attribute: name of parent does not work

2ef1b5f

[os_xml] _ReadElem: remove unused recursive parameter

3946ce1

[os_xml] better handling of whitespaces (' ', '\n', '\t') while parsi…

450e387

…ng xml node attributes

[os_xml] use isspace() instead of manual checking

3fd78d7

fixed #149

61504f9

Merge pull request #3 from loonhxl/os_xml

1cbbec6

fixed #149

[os_xml] add testcases for pr #3 (thanls to @loonhxl)

a92a49c

jrossi added a commit that referenced this pull request Mar 20, 2014

Merge pull request #142 from cgzones/os_xml

ff90fc3

os_xml review

jrossi merged commit ff90fc3 into ossec:master Mar 20, 2014

cgzones deleted the os_xml branch March 21, 2014 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os_xml review #142

os_xml review #142

cgzones commented Mar 15, 2014

jrossi commented Mar 15, 2014

cgzones commented Mar 15, 2014

jrossi commented Mar 18, 2014

jrossi commented Mar 20, 2014

os_xml review #142

os_xml review #142

Conversation

cgzones commented Mar 15, 2014

jrossi commented Mar 15, 2014

cgzones commented Mar 15, 2014

jrossi commented Mar 18, 2014

jrossi commented Mar 20, 2014