Blob Blame History Raw
From e18cd8da50b447de12376713e92be798d4327464 Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 07:55:53 +0100
Subject: [PATCH 1/6] Fix resource leaks and null pointer dereferences

Fixes memory leaks and file descriptor leaks in error handling code.
Fixes null pointer dereferences.
---
 libcomps/src/comps_bradix.c             | 15 ++++++++---
 libcomps/src/comps_elem.c               |  7 ++++-
 libcomps/src/comps_hslist.c             |  3 ++-
 libcomps/src/comps_mradix.c             |  9 +++++--
 libcomps/src/comps_objmradix.c          |  5 +++-
 libcomps/src/comps_objradix.c           |  9 +++++--
 libcomps/src/python/src/pycomps_dict.c  | 34 +++++++++++++++++-------
 libcomps/src/python/src/pycomps_mdict.c | 35 ++++++++++++++++++-------
 libcomps/tests/check_objrtree.c         |  6 ++++-
 libcomps/tests/check_rtree.c            | 10 +++++--
 10 files changed, 101 insertions(+), 32 deletions(-)

diff --git a/libcomps/src/comps_bradix.c b/libcomps/src/comps_bradix.c
index 46488b5..3d442e2 100644
--- a/libcomps/src/comps_bradix.c
+++ b/libcomps/src/comps_bradix.c
@@ -40,7 +40,10 @@ inline COMPS_BRTreeData * __comps_brtree_data_create(COMPS_BRTree *brt,
     if ((brtd = malloc(sizeof(COMPS_BRTreeData))) == NULL)
         return NULL;
     brtd->key = brt->key_clone(key, keylen);
-    if (!brtd->key) return NULL;
+    if (!brtd->key) {
+        free(brtd);
+        return NULL;
+    }
 
     brtd->data = data;
     if (data != NULL) {
@@ -305,7 +308,6 @@ void comps_brtree_set(COMPS_BRTree * brt, void * key, void * data)
                 it->next = NULL;
                 ((COMPS_BRTreeData*)subnodes->last->data)->subnodes->last->next = it;
                 ((COMPS_BRTreeData*)subnodes->last->data)->subnodes->last = it;
-                len = brt->key_len(brtdata->key)-x;
                 newkey = brt->subkey(brtdata->key, x, brt->key_len(brtdata->key));
                 brt->key_destroy(brtdata->key);
                 brtdata->key = newkey;
@@ -415,8 +417,10 @@ void comps_brtree_unset(COMPS_BRTree * brt, void * key) {
                 break;
             }
         }
-        if (!found)
+        if (!found) {
+            comps_hslist_destroy(&path);
             return;
+        }
         brtdata = (COMPS_BRTreeData*)it->data;
         x = brt->key_cmp(brtdata->key, key, 1, offset+1, len, &ended);
         if (ended == 3) {
@@ -459,7 +463,10 @@ void comps_brtree_unset(COMPS_BRTree * brt, void * key) {
             return;
         }
         else if (ended == 1) offset+=x;
-        else return;
+        else {
+            comps_hslist_destroy(&path);
+            return;
+        }
         if ((relation = malloc(sizeof(struct Relation))) == NULL) {
             comps_hslist_destroy(&path);
             return;
diff --git a/libcomps/src/comps_elem.c b/libcomps/src/comps_elem.c
index 359fbd0..38fed3c 100644
--- a/libcomps/src/comps_elem.c
+++ b/libcomps/src/comps_elem.c
@@ -915,7 +915,12 @@ COMPS_Elem* comps_elem_create(const char * s, const char ** attrs,
     if (attrs != NULL) {
         for (; *attrs != NULL; attrs += 2) {
             val = malloc((strlen(*(attrs+1))+1)*sizeof(char));
-            if (val == NULL) return NULL;
+            if (val == NULL) {
+                comps_dict_destroy(elem->attrs);
+                free(elem->name);
+                free(elem);
+                return NULL;
+            }
             memcpy(val, *(attrs+1), sizeof(char) * (strlen(*(attrs+1))+1));
             comps_dict_set(elem->attrs, (char*)*attrs, val);
         }
diff --git a/libcomps/src/comps_hslist.c b/libcomps/src/comps_hslist.c
index cc81d95..13a27a1 100644
--- a/libcomps/src/comps_hslist.c
+++ b/libcomps/src/comps_hslist.c
@@ -71,7 +71,7 @@ void comps_hslist_insert_after(COMPS_HSList * hslist, COMPS_HSListItem *item,
     COMPS_HSListItem * it;
     void * ndata;
 
-    if (hslist == NULL && item == NULL)
+    if (hslist == NULL || item == NULL)
         return;
     if ((it = malloc(sizeof(*it))) == NULL)
         return;
@@ -206,6 +206,7 @@ void comps_hslist_destroy(COMPS_HSList ** hslist) {
 void comps_hslist_remove(COMPS_HSList * hslist,
                               COMPS_HSListItem * it) {
     COMPS_HSListItem *itx, *itprev=NULL;
+    if (it == NULL) return;
     for (itx = hslist->first; itx != NULL && itx != it; itx = itx->next) {
         itprev = itx;
     }
diff --git a/libcomps/src/comps_mradix.c b/libcomps/src/comps_mradix.c
index 338cb07..8ef9640 100644
--- a/libcomps/src/comps_mradix.c
+++ b/libcomps/src/comps_mradix.c
@@ -532,8 +532,10 @@ void comps_mrtree_unset(COMPS_MRTree * rt, const char * key) {
                 break;
             }
         }
-        if (!found)
+        if (!found) {
+            comps_hslist_destroy(&path);
             return;
+        }
         rtdata = (COMPS_MRTreeData*)it->data;
 
         for (x=1; ;x++) {
@@ -582,7 +584,10 @@ void comps_mrtree_unset(COMPS_MRTree * rt, const char * key) {
             return;
         }
         else if (ended == 1) offset+=x;
-        else return;
+        else {
+            comps_hslist_destroy(&path);
+            return;
+        }
         if ((relation = malloc(sizeof(struct Relation))) == NULL) {
             comps_hslist_destroy(&path);
             return;
diff --git a/libcomps/src/comps_objmradix.c b/libcomps/src/comps_objmradix.c
index 9be6648..9a2038b 100644
--- a/libcomps/src/comps_objmradix.c
+++ b/libcomps/src/comps_objmradix.c
@@ -585,7 +585,10 @@ void comps_objmrtree_unset(COMPS_ObjMRTree * rt, const char * key) {
             return;
         }
         else if (ended == 1) offset+=x;
-        else return;
+        else {
+            comps_hslist_destroy(&path);
+            return;
+        }
         if ((relation = malloc(sizeof(struct Relation))) == NULL) {
             comps_hslist_destroy(&path);
             return;
diff --git a/libcomps/src/comps_objradix.c b/libcomps/src/comps_objradix.c
index a790270..c657b75 100644
--- a/libcomps/src/comps_objradix.c
+++ b/libcomps/src/comps_objradix.c
@@ -540,8 +540,10 @@ void comps_objrtree_unset(COMPS_ObjRTree * rt, const char * key) {
                 break;
             }
         }
-        if (!found)
+        if (!found) {
+            comps_hslist_destroy(&path);
             return;
+        }
         rtdata = (COMPS_ObjRTreeData*)it->data;
 
         for (x=1; ;x++) {
@@ -591,7 +593,10 @@ void comps_objrtree_unset(COMPS_ObjRTree * rt, const char * key) {
             return;
         }
         else if (ended == 1) offset+=x;
-        else return;
+        else {
+            comps_hslist_destroy(&path);
+            return;
+        }
         if ((relation = malloc(sizeof(struct Relation))) == NULL) {
             comps_hslist_destroy(&path);
             return;
diff --git a/libcomps/src/python/src/pycomps_dict.c b/libcomps/src/python/src/pycomps_dict.c
index 52ce854..1b42909 100644
--- a/libcomps/src/python/src/pycomps_dict.c
+++ b/libcomps/src/python/src/pycomps_dict.c
@@ -73,7 +73,7 @@ int PyCOMPSDict_init(PyCOMPS_Dict *self, PyObject *args, PyObject *kwds)
 PyObject* PyCOMPSDict_str(PyObject *self) {
     COMPS_HSList *pairlist;
     COMPS_HSListItem *it;
-    PyObject *ret, *tmp, *tmp2, *tmpkey, *tmpval;
+    PyObject *ret, *tmp = NULL, *tmp2 = NULL, *tmpkey = NULL, *tmpval = NULL;
     ret = PyUnicode_FromString("{");
     pairlist = comps_objdict_pairs(((PyCOMPS_Dict*)self)->dict);
     char *tmpstr;
@@ -83,14 +83,14 @@ PyObject* PyCOMPSDict_str(PyObject *self) {
         tmpkey = __pycomps_lang_decode(((COMPS_ObjRTreePair*)it->data)->key);
         if (!tmpkey) {
             PyErr_SetString(PyExc_TypeError, "key convert error");
-            return NULL;
+            goto out;
         }
         tmpstr = comps_object_tostr(((COMPS_ObjRTreePair*)it->data)->data);
         tmpval = __pycomps_lang_decode(tmpstr);
         free(tmpstr);
         if (!tmpval) {
             PyErr_SetString(PyExc_TypeError, "val convert error");
-            return NULL;
+            goto out;
         }
         tmp2 = PyUnicode_FromFormat("%U = '%U', ", tmpkey, tmpval);
         ret = PyUnicode_Concat(ret, tmp2);
@@ -102,14 +102,14 @@ PyObject* PyCOMPSDict_str(PyObject *self) {
     tmp = ret;
     tmpkey = __pycomps_lang_decode(((COMPS_RTreePair*)it->data)->key);
     if (!tmpkey) {
-        return NULL;
+        goto out;
     }
     tmpstr = comps_object_tostr(((COMPS_ObjRTreePair*)it->data)->data);
     tmpval = __pycomps_lang_decode(tmpstr);
     free(tmpstr);
     if (!tmpval) {
         //PyErr_SetString(PyExc_TypeError, "val convert error");
-        return NULL;
+        goto out;
     }
     tmp2 = PyUnicode_FromFormat("%U = '%U'", tmpkey, tmpval);
     ret = PyUnicode_Concat(ret, tmp2);
@@ -126,6 +126,14 @@ PyObject* PyCOMPSDict_str(PyObject *self) {
 
     comps_hslist_destroy(&pairlist);
     return ret;
+
+    out:
+    Py_XDECREF(tmp);
+    Py_XDECREF(tmp2);
+    Py_XDECREF(tmpkey);
+    Py_XDECREF(tmpval);
+    comps_hslist_destroy(&pairlist);
+    return NULL;
 }
 
 int PyCOMPSDict_print(PyObject *self, FILE *f, int flags) {
@@ -155,8 +163,12 @@ int PyCOMPSDict_print(PyObject *self, FILE *f, int flags) {
 
 PyObject* PyCOMPSDict_cmp(PyObject *self, PyObject *other, int op) {
     char ret;
-    if (other == NULL || (Py_TYPE(other) != Py_TYPE(self) &&
-                          !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
+    if (other == NULL) {
+        PyErr_Format(PyExc_TypeError, "Get NULL as Dict subclass");
+        return NULL;
+    }
+    if ((Py_TYPE(other) != Py_TYPE(self) &&
+         !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
         PyErr_Format(PyExc_TypeError, "Not Dict subclass, %s",
                                       Py_TYPE(other)->tp_name);
         return NULL;
@@ -190,8 +202,12 @@ PyObject* PyCOMPSDict_copy(PyObject *self) {
     return ret;
 }
 PyObject* PyCOMPSDict_update(PyObject *self, PyObject *other) {
-    if (other == NULL || (Py_TYPE(other) != Py_TYPE(self) &&
-                          !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
+    if (other == NULL) {
+        PyErr_Format(PyExc_TypeError, "Get NULL as Dict subclass");
+        return NULL;
+    }
+    if ((Py_TYPE(other) != Py_TYPE(self) &&
+         !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
         PyErr_Format(PyExc_TypeError, "Not %s type or subclass, %s",
                      Py_TYPE(self)->tp_name, Py_TYPE(other)->tp_name);
         return NULL;
diff --git a/libcomps/src/python/src/pycomps_mdict.c b/libcomps/src/python/src/pycomps_mdict.c
index e98f414..2830ed8 100644
--- a/libcomps/src/python/src/pycomps_mdict.c
+++ b/libcomps/src/python/src/pycomps_mdict.c
@@ -69,7 +69,7 @@ PyObject* PyCOMPSMDict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 PyObject* PyCOMPSMDict_str(PyObject *self) {
     COMPS_HSList *pairlist;
     COMPS_HSListItem *it;
-    PyObject *ret, *tmp, *tmp2, *tmpkey, *tmpval;
+    PyObject *ret, *tmp = NULL, *tmp2 = NULL, *tmpkey = NULL, *tmpval = NULL;
     ret = PyUnicode_FromString("{");
     pairlist = comps_objmdict_pairs(((PyCOMPS_MDict*)self)->dict);
     char *tmpstr;
@@ -79,7 +79,7 @@ PyObject* PyCOMPSMDict_str(PyObject *self) {
         tmpkey = __pycomps_lang_decode(((COMPS_ObjMRTreePair*)it->data)->key);
         if (!tmpkey) {
             PyErr_SetString(PyExc_TypeError, "key convert error");
-            return NULL;
+            goto out;
         }
         tmpstr = comps_object_tostr((COMPS_Object*)
                                     ((COMPS_ObjMRTreePair*)it->data)->data);
@@ -87,7 +87,7 @@ PyObject* PyCOMPSMDict_str(PyObject *self) {
         free(tmpstr);
         if (!tmpval) {
             PyErr_SetString(PyExc_TypeError, "val convert error");
-            return NULL;
+            goto out;
         }
         tmp2 = PyUnicode_FromFormat("%U = '%U', ", tmpkey, tmpval);
         ret = PyUnicode_Concat(ret, tmp2);
@@ -100,7 +100,7 @@ PyObject* PyCOMPSMDict_str(PyObject *self) {
     tmpkey = __pycomps_lang_decode(((COMPS_ObjMRTreePair*)it->data)->key);
     if (!tmpkey) {
         //PyErr_SetString(PyExc_TypeError, "key convert error");
-        return NULL;
+        goto out;
     }
     tmpstr = comps_object_tostr((COMPS_Object*)
                                 ((COMPS_ObjMRTreePair*)it->data)->data);
@@ -110,7 +110,7 @@ PyObject* PyCOMPSMDict_str(PyObject *self) {
     free(tmpstr);
     if (!tmpval) {
         //PyErr_SetString(PyExc_TypeError, "val convert error");
-        return NULL;
+        goto out;
     }
     tmp2 = PyUnicode_FromFormat("%U = '%U'", tmpkey, tmpval);
     ret = PyUnicode_Concat(ret, tmp2);
@@ -127,6 +127,15 @@ PyObject* PyCOMPSMDict_str(PyObject *self) {
 
     comps_hslist_destroy(&pairlist);
     return ret;
+
+    out:
+    Py_XDECREF(tmp);
+    Py_XDECREF(tmp2);
+    Py_XDECREF(tmpkey);
+    Py_XDECREF(tmpval);
+    comps_hslist_destroy(&pairlist);
+    return NULL;
+
 }
 
 int PyCOMPSMDict_print(PyObject *self, FILE *f, int flags) {
@@ -159,8 +168,12 @@ int PyCOMPSMDict_print(PyObject *self, FILE *f, int flags) {
 
 PyObject* PyCOMPSMDict_cmp(PyObject *self, PyObject *other, int op) {
     char ret;
-    if (other == NULL || (Py_TYPE(other) != Py_TYPE(self) &&
-                          !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
+    if (other == NULL) {
+        PyErr_Format(PyExc_TypeError, "Get NULL as Dict subclass");
+        return NULL;
+    }
+    if ((Py_TYPE(other) != Py_TYPE(self) &&
+         !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
         PyErr_Format(PyExc_TypeError, "Not Dict subclass, %s",
                                       Py_TYPE(other)->tp_name);
         return NULL;
@@ -194,8 +207,12 @@ PyObject* PyCOMPSMDict_copy(PyObject *self) {
     return ret;
 }
 PyObject* PyCOMPSMDict_update(PyObject *self, PyObject *other) {
-    if (other == NULL || (Py_TYPE(other) != Py_TYPE(self) &&
-                          !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
+    if (other == NULL) {
+        PyErr_Format(PyExc_TypeError, "Get NULL as Dict subclass");
+        return NULL;
+    }
+    if ((Py_TYPE(other) != Py_TYPE(self) &&
+         !PyType_IsSubtype(Py_TYPE(other), Py_TYPE(self)))) {
         PyErr_Format(PyExc_TypeError, "Not %s type or subclass, %s",
                      Py_TYPE(self)->tp_name, Py_TYPE(other)->tp_name);
         return NULL;
diff --git a/libcomps/tests/check_objrtree.c b/libcomps/tests/check_objrtree.c
index 7a12f9c..bdb3f0e 100644
--- a/libcomps/tests/check_objrtree.c
+++ b/libcomps/tests/check_objrtree.c
@@ -54,10 +54,14 @@ COMPS_ObjRTree * load_acrodict(char *filename) {
     if (!rt) return NULL;
 
     f = fopen(filename, "r");
-    if (!f) return NULL;
+    if (!f) {
+        COMPS_OBJECT_DESTROY(rt);
+        return NULL;
+    }
     while (!feof(f)) {
         memset(buffer, 0, 100);
         if (!fgets(buffer, 100, f))
+            fclose(f);
             return rt;
         buffer[strlen(buffer)-1] =0;
         //printf("buffer:%s\n", buffer);
diff --git a/libcomps/tests/check_rtree.c b/libcomps/tests/check_rtree.c
index 1dbf45d..66e0075 100644
--- a/libcomps/tests/check_rtree.c
+++ b/libcomps/tests/check_rtree.c
@@ -96,11 +96,16 @@ COMPS_RTree * load_acrodict(char *filename) {
     if (!rt) return NULL;
 
     f = fopen(filename, "r");
-    if (!f) return NULL;
+    if (!f) {
+        comps_rtree_destroy(rt);
+        return NULL;
+    }
     while (!feof(f)) {
         memset(buffer, 0, 100);
-        if (!fgets(buffer, 100, f))
+        if (!fgets(buffer, 100, f)) {
+            fclose(f);
             return rt;
+        }
         buffer[strlen(buffer)-1] =0;
         //printf("buffer:%s\n", buffer);
         pch = strrchr(buffer, '-');
@@ -114,6 +119,7 @@ COMPS_RTree * load_acrodict(char *filename) {
             printf("%s\n", ((COMPS_RTreeData*)it->data)->key);
         }*/
     }
+    fclose(f);
     return rt;
 }
 

From 5e216a6b231118edf39d57a8c2ed4b189e1279cd Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 08:08:32 +0100
Subject: [PATCH 2/6] Fix memory "double free"

---
 libcomps/src/comps_doc.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/libcomps/src/comps_doc.c b/libcomps/src/comps_doc.c
index 806a363..9e6005b 100644
--- a/libcomps/src/comps_doc.c
+++ b/libcomps/src/comps_doc.c
@@ -734,7 +734,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
     if (mdict && mdict->len) {
         retc = xmlTextWriterStartElement(writer, BAD_CAST "blacklist");
         if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-            COMPS_OBJECT_DESTROY(dict);
             return -1;
         }
         hslist = comps_objmrtree_pairs(mdict);
@@ -744,7 +743,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
                  it != NULL; it = it->next) {
                 retc = xmlTextWriterStartElement(writer, BAD_CAST "package");
                 if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-                    COMPS_OBJECT_DESTROY(dict);
                     comps_hslist_destroy(&hslist);
                     return -1;
                 }
@@ -757,7 +755,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
 
                 retc = xmlTextWriterEndElement(writer);
                 if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-                    COMPS_OBJECT_DESTROY(dict);
                     comps_hslist_destroy(&hslist);
                     return -1;
                 }
@@ -767,7 +764,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
 
         retc = xmlTextWriterEndElement(writer);
         if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-            COMPS_OBJECT_DESTROY(dict);
             return -1;
         }
     }
@@ -786,7 +782,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
                  it != NULL; it = it->next) {
                 retc = xmlTextWriterStartElement(writer, BAD_CAST "ignoredep");
                 if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-                    COMPS_OBJECT_DESTROY(dict);
                     comps_hslist_destroy(&hslist);
                     return -1;
                 }
@@ -800,7 +795,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
 
                 retc = xmlTextWriterEndElement(writer);
                 if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-                    COMPS_OBJECT_DESTROY(dict);
                     comps_hslist_destroy(&hslist);
                     return -1;
                 }
@@ -810,7 +804,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer,
 
         retc = xmlTextWriterEndElement(writer);
         if (__comps_check_xml_get(retc, (COMPS_Object*)doc->log) < 0) {
-            COMPS_OBJECT_DESTROY(dict);
             return -1;
         }
     }

From eef385add088fe0307f1bb9fbb4e1ba3abfd4fcd Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 08:18:51 +0100
Subject: [PATCH 3/6] Fix memory leaks, modify macro COMPS_XMLRET_CHECK

Macro COMPS_XMLRET_CHECK was modified to support user code to resource freeing.
It is used to fix memory leaks.
---
 libcomps/src/comps_doccategory.c | 10 +++++-----
 libcomps/src/comps_docenv.c      | 16 ++++++++--------
 libcomps/src/comps_docgroup.c    | 18 +++++++++---------
 libcomps/src/comps_docgroupid.c  | 10 +++++-----
 libcomps/src/comps_docpackage.c  | 12 ++++++------
 libcomps/src/comps_utils.h       |  3 ++-
 6 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/libcomps/src/comps_doccategory.c b/libcomps/src/comps_doccategory.c
index 6c60826..9c36633 100644
--- a/libcomps/src/comps_doccategory.c
+++ b/libcomps/src/comps_doccategory.c
@@ -230,12 +230,12 @@ signed char comps_doccategory_xml(COMPS_DocCategory *category,
         return 1;
     }
     ret = xmlTextWriterStartElement(writer, BAD_CAST "category");
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     if (xml_options->arch_output) {
         obj = (COMPS_Object*)comps_doccategory_arches(category);
         ret = __comps_xml_arch(obj, writer);
         COMPS_OBJECT_DESTROY(obj);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
     for (int i=0; i<6; i++) {
         if (!type[i]) {
@@ -281,16 +281,16 @@ signed char comps_doccategory_xml(COMPS_DocCategory *category,
     }
     if (category->group_ids->len || xml_options->empty_grouplist) {
         ret = xmlTextWriterStartElement(writer, (xmlChar*)"grouplist");
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         for (it = category->group_ids->first; it != NULL; it = it->next) {
             comps_docgroupid_xml((COMPS_DocGroupId*)(COMPS_DocGroupId*)it->comps_obj,
                                  writer, log, xml_options, def_options);
         }
         ret = xmlTextWriterEndElement(writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
     ret = xmlTextWriterEndElement(writer);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     return 0;
 }
 
diff --git a/libcomps/src/comps_docenv.c b/libcomps/src/comps_docenv.c
index d34055b..f11dfa1 100644
--- a/libcomps/src/comps_docenv.c
+++ b/libcomps/src/comps_docenv.c
@@ -204,7 +204,7 @@ COMPS_DocEnv* comps_docenv_union(COMPS_DocEnv *e1, COMPS_DocEnv *e2) {
         if ((data = comps_set_data_at(set, (void*)it->comps_obj)) != NULL) {
             index = comps_objlist_index(res->option_list, (COMPS_Object*)data);
             comps_objlist_remove_at(res->option_list, index);
-            ret = comps_objlist_insert_at_x(res->option_list, index,
+            comps_objlist_insert_at_x(res->option_list, index,
                                       comps_object_copy(it->comps_obj));
         } else {
             comps_docenv_add_optionid(res, (COMPS_DocGroupId*)
@@ -314,11 +314,11 @@ signed char comps_docenv_xml(COMPS_DocEnv *env, xmlTextWriterPtr writer,
         return 1;
     }
     ret = xmlTextWriterStartElement(writer, BAD_CAST "environment");
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     if (xml_options->arch_output) {
         obj = (COMPS_Object*)comps_docenv_arches(env);
         ret = __comps_xml_arch(obj, writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         COMPS_OBJECT_DESTROY(obj);
     }
     for (int i=0; i<6; i++) {
@@ -365,27 +365,27 @@ signed char comps_docenv_xml(COMPS_DocEnv *env, xmlTextWriterPtr writer,
     }
     if (env->group_list->len || xml_options->empty_grouplist) {
         ret = xmlTextWriterStartElement(writer, (xmlChar*)"grouplist");
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         for (it = env->group_list->first; it != NULL; it = it->next) {
             comps_docgroupid_xml((COMPS_DocGroupId*)(COMPS_DocGroupId*)it->comps_obj,
                                  writer, log, xml_options, def_options);
         }
         ret = xmlTextWriterEndElement(writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
 
     if (env->option_list->len || xml_options->empty_optionlist) {
         ret = xmlTextWriterStartElement(writer, (xmlChar*)"optionlist");
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         for (it = env->option_list->first; it != NULL; it = it->next) {
             comps_docgroupid_xml((COMPS_DocGroupId*)(COMPS_DocGroupId*)it->comps_obj,
                                  writer, log, xml_options, def_options);
         }
         ret = xmlTextWriterEndElement(writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
     ret = xmlTextWriterEndElement(writer);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     return 0;
 }
 
diff --git a/libcomps/src/comps_docgroup.c b/libcomps/src/comps_docgroup.c
index f305ffd..6c0eb14 100644
--- a/libcomps/src/comps_docgroup.c
+++ b/libcomps/src/comps_docgroup.c
@@ -305,11 +305,11 @@ signed char comps_docgroup_xml(COMPS_DocGroup *group, xmlTextWriterPtr writer,
         return 1;
     }
     ret = xmlTextWriterStartElement(writer, BAD_CAST "group");
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     if (xml_options->arch_output) {
         obj = (COMPS_Object*)comps_docgroup_arches(group);
         ret = __comps_xml_arch(obj, writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         COMPS_OBJECT_DESTROY(obj);
     }
     for (int i=0; i<10; i++) {
@@ -344,33 +344,33 @@ signed char comps_docgroup_xml(COMPS_DocGroup *group, xmlTextWriterPtr writer,
             for (hsit = pairlist->first; hsit != NULL; hsit = hsit->next) {
                 ret = xmlTextWriterStartElement(writer,
                             (const xmlChar*)((aliases[i])?aliases[i]:props[i]));
-                COMPS_XMLRET_CHECK
+                COMPS_XMLRET_CHECK(comps_hslist_destroy(&pairlist))
 
                 ret = xmlTextWriterWriteAttribute(writer, BAD_CAST "xml:lang",
                             (xmlChar*) ((COMPS_ObjRTreePair*)hsit->data)->key);
-                COMPS_XMLRET_CHECK
+                COMPS_XMLRET_CHECK(comps_hslist_destroy(&pairlist))
                 str = tostrf[i](((COMPS_ObjRTreePair*)hsit->data)->data);
                 ret = xmlTextWriterWriteString(writer, (xmlChar*)str);
-                COMPS_XMLRET_CHECK
+                COMPS_XMLRET_CHECK(comps_hslist_destroy(&pairlist))
                 free(str);
                 ret = xmlTextWriterEndElement(writer);
-                COMPS_XMLRET_CHECK
+                COMPS_XMLRET_CHECK(comps_hslist_destroy(&pairlist))
             }
             comps_hslist_destroy(&pairlist);
         }
     }
     if (group->packages->len || xml_options->empty_packages) {
         ret = xmlTextWriterStartElement(writer, (xmlChar*)"packagelist");
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
         for (it = group->packages->first; it != NULL; it = it->next) {
             comps_docpackage_xml((COMPS_DocGroupPackage*)it->comps_obj,
                                  writer, log, xml_options, def_options);
         }
         ret = xmlTextWriterEndElement(writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
     ret = xmlTextWriterEndElement(writer);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     return 0;
 }
 
diff --git a/libcomps/src/comps_docgroupid.c b/libcomps/src/comps_docgroupid.c
index 741b229..eefd42f 100644
--- a/libcomps/src/comps_docgroupid.c
+++ b/libcomps/src/comps_docgroupid.c
@@ -109,13 +109,13 @@ signed char comps_docgroupid_xml(COMPS_DocGroupId *groupid,
     (void)def_options;
 
     ret = xmlTextWriterStartElement(writer, BAD_CAST "groupid");
+    COMPS_XMLRET_CHECK()
     if (options->arch_output) {
         COMPS_Object *obj = (COMPS_Object*)groupid->arches;
         //printf("obj %d\n", obj);
         ret = __comps_xml_arch(obj, writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
-    COMPS_XMLRET_CHECK
     if (options->gid_default_explicit) {
         if (groupid->def)
             ret = xmlTextWriterWriteAttribute(writer, BAD_CAST "default",
@@ -123,7 +123,7 @@ signed char comps_docgroupid_xml(COMPS_DocGroupId *groupid,
         else
             ret = xmlTextWriterWriteAttribute(writer, BAD_CAST "default",
                                               BAD_CAST "false");
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     } else if (groupid->def != default_def) {
         if (groupid->def)
             ret = xmlTextWriterWriteAttribute(writer, BAD_CAST "default",
@@ -134,10 +134,10 @@ signed char comps_docgroupid_xml(COMPS_DocGroupId *groupid,
     }
     str = comps_object_tostr((COMPS_Object*)groupid->name);
     ret = xmlTextWriterWriteString(writer, BAD_CAST str);
-    COMPS_XMLRET_CHECK
     free(str);
+    COMPS_XMLRET_CHECK()
     ret = xmlTextWriterEndElement(writer);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     return 0;
 }
 
diff --git a/libcomps/src/comps_docpackage.c b/libcomps/src/comps_docpackage.c
index 449ef97..25d8564 100644
--- a/libcomps/src/comps_docpackage.c
+++ b/libcomps/src/comps_docpackage.c
@@ -150,11 +150,11 @@ signed char comps_docpackage_xml(COMPS_DocGroupPackage *pkg,
     (void)def_options;
 
     ret = xmlTextWriterStartElement(writer, BAD_CAST "packagereq");
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     if (xml_options->arch_output) {
         COMPS_Object *obj = (COMPS_Object*)pkg->arches;
         ret = __comps_xml_arch(obj, writer);
-        COMPS_XMLRET_CHECK
+        COMPS_XMLRET_CHECK()
     }
     if (pkg->type == COMPS_PACKAGE_OPTIONAL)
         str = "optional";
@@ -172,7 +172,7 @@ signed char comps_docpackage_xml(COMPS_DocGroupPackage *pkg,
                                             BAD_CAST str);
         free(str);
     }
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     if (xml_options->bao_explicit) {
         if (pkg->basearchonly) {
             ret = xmlTextWriterWriteAttribute(writer, (xmlChar*) "basearchonly",
@@ -192,13 +192,13 @@ signed char comps_docpackage_xml(COMPS_DocGroupPackage *pkg,
             }
         }
     }
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     str = comps_object_tostr((COMPS_Object*)pkg->name);
     ret = xmlTextWriterWriteString(writer, (xmlChar*)str);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     free(str);
     ret = xmlTextWriterEndElement(writer);
-    COMPS_XMLRET_CHECK
+    COMPS_XMLRET_CHECK()
     return 0;
 }
 
diff --git a/libcomps/src/comps_utils.h b/libcomps/src/comps_utils.h
index e825f94..de59e63 100644
--- a/libcomps/src/comps_utils.h
+++ b/libcomps/src/comps_utils.h
@@ -172,7 +172,8 @@ void CONCAT(CONCAT(comps_, OBJ), _set_arches)(OBJTYPE *obj,\
                                               COMPS_ObjList *list);
 
 
-#define COMPS_XMLRET_CHECK if (ret == -1) {\
+#define COMPS_XMLRET_CHECK(free_code) if (ret == -1) {\
+    free_code;\
     comps_log_error(log, COMPS_ERR_XMLGEN, 0);\
     return -1;\
 }

From 93df35852eec7b07974d6553517233f802041d0d Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 08:37:45 +0100
Subject: [PATCH 4/6] Add tests of null pointers and optimize code

---
 libcomps/src/comps_objlist.c               | 89 +++++++++-------------
 libcomps/src/comps_parse.c                 |  6 +-
 libcomps/src/comps_set.c                   |  5 +-
 libcomps/src/python/src/pycomps_sequence.c | 10 ++-
 libcomps/tests/check_utils.c               |  6 +-
 5 files changed, 48 insertions(+), 68 deletions(-)

diff --git a/libcomps/src/comps_objlist.c b/libcomps/src/comps_objlist.c
index 00f42a8..3ce6beb 100644
--- a/libcomps/src/comps_objlist.c
+++ b/libcomps/src/comps_objlist.c
@@ -58,10 +58,8 @@ void comps_objlist_destroy(COMPS_ObjList *objlist) {
     COMPS_ObjListIt *oldit, *it = objlist->first;
     COMPS_Object **obj= NULL;
 
-    oldit = it;
-    for (; comps_objlist_walk(&it, obj); oldit = it) {
+    for (oldit = it; comps_objlist_walk(&it, obj); oldit = it) {
         comps_objlist_it_destroy(oldit);
-        oldit = it;
     }
     if (oldit)
         comps_objlist_it_destroy(oldit);
@@ -143,7 +141,7 @@ int comps_objlist_walk_r(COMPS_ObjListIt *start,
 }
 
 static int __comps_objlist_append(COMPS_ObjList *objlist, COMPS_ObjListIt *objit) {
-    if (!objlist) return 0;
+    if (!objlist || !objit) return 0;
     if (objlist->first == NULL) {
         objlist->first = objit;
         objlist->last = objit;
@@ -215,12 +213,10 @@ int __comps_objlist_insert_at(COMPS_ObjList *objlist,
         if (!objlist->last) {
             objlist->last = newit;
         }
-        objlist->len++;
     } else if (pos == objlist->len){
         newit->next = NULL;
         objlist->last->next = newit;
         objlist->last = newit;
-        objlist->len++;
     } else {
         i = 0;
         oldit = NULL;
@@ -230,8 +226,8 @@ int __comps_objlist_insert_at(COMPS_ObjList *objlist,
         }
         newit->next = oldit->next;
         oldit->next = newit; 
-        objlist->len++;
     }
+    objlist->len++;
     return 1;
 }
 int comps_objlist_insert_at_x(COMPS_ObjList *objlist,
@@ -253,7 +249,7 @@ int comps_objlist_insert_at(COMPS_ObjList *objlist,
 
 int comps_objlist_remove_at(COMPS_ObjList *objlist, unsigned int atpos) {
     int pos;
-    COMPS_ObjListIt *it, *itprev=NULL, *removed=NULL;
+    COMPS_ObjListIt *it, *itprev = NULL;
     if (!objlist) return 0;
 
     for (it = objlist->first, pos=0;
@@ -261,31 +257,20 @@ int comps_objlist_remove_at(COMPS_ObjList *objlist, unsigned int atpos) {
          it = it->next, pos++) {
         itprev = it;
     }
-    if (atpos == 0 && objlist->first) {
-        removed = objlist->first;
-        objlist->first = objlist->first->next;
-        if (objlist->last == removed) {
-            objlist->last = NULL;
-        }
-    } else if (pos != (int)(atpos))
+    if (!it)
         return 0;
-    if (itprev) {
-        removed = it;
-        if (itprev->next)
-            itprev->next = itprev->next->next;
-        else
-            itprev->next = NULL;
-        if (removed == objlist->last) {
-            objlist->last = itprev;
-        }
-    }
-    comps_objlist_it_destroy(removed);
+    if (itprev)
+        itprev->next = it->next;
+    else
+        objlist->first = it->next;
+    if (it == objlist->last)
+        objlist->last = itprev;
+    comps_objlist_it_destroy(it);
     objlist->len--;
     return 1;
 }
 
 int comps_objlist_remove(COMPS_ObjList *objlist, COMPS_Object *obj) {
-    //int pos;
     COMPS_ObjListIt *it, *itprev = NULL;
     if (!objlist) return 0;
 
@@ -293,23 +278,15 @@ int comps_objlist_remove(COMPS_ObjList *objlist, COMPS_Object *obj) {
          it = it->next) {
         itprev = it;
     }
-    if (it == NULL)
+    if (!it)
         return 0;
-
-    if (itprev == NULL && it->comps_obj == obj) {
-        if (objlist->last == objlist->first)
-            objlist->last = NULL;
-        objlist->first = objlist->first->next;
-        comps_object_destroy(it->comps_obj);
-        free(it);
-    } else {
+    if (itprev)
         itprev->next = it->next;
-        comps_object_destroy(it->comps_obj);
-        free(it);
-        if (it == objlist->last) {
-            objlist->last = itprev;
-        }
-    }
+    else
+        objlist->first = it->next;
+    if (it == objlist->last)
+        objlist->last = itprev;
+    comps_objlist_it_destroy(it);
     objlist->len--;
     return 1;
 }
@@ -366,6 +343,8 @@ COMPS_ObjList* comps_objlist_sublist_indexed(COMPS_ObjList *objlist,
     for (it = objlist->first, pos=0;
          it != NULL && pos != start;
          it = it->next, pos++);
+    if (!it)
+        return ret;
     for (; it->next != NULL && pos != end; it = it->next, pos++) {
         comps_objlist_append(ret, it->comps_obj);
     }
@@ -387,6 +366,8 @@ COMPS_ObjList* comps_objlist_sublist_indexed_step(COMPS_ObjList *objlist,
     for (it = objlist->first;
          it != NULL && pos != start;
          it = it->next, pos++);
+    if (!it)
+        return ret;
     for (; it->next != NULL && pos != end; it = it->next, pos++, stepc++) {
         if (stepc == step) {
             step = 0;
@@ -471,12 +452,12 @@ char* comps_objlist_tostr_u(COMPS_Object* list) {
     const int sep_len = strlen(", ");
 
     COMPS_ObjListIt *it = ((COMPS_ObjList*)list)->first;
-    for (; it != ((COMPS_ObjList*)list)->last; it = it->next, i++) {
-        tmps = comps_object_tostr(it->comps_obj);
-        items[i] = tmps;
-        total_strlen += sep_len + strlen(tmps);
-    }
     if (it) {
+        for (; it != ((COMPS_ObjList*)list)->last; it = it->next, i++) {
+            tmps = comps_object_tostr(it->comps_obj);
+            items[i] = tmps;
+            total_strlen += sep_len + strlen(tmps);
+        }
         tmps = comps_object_tostr(it->comps_obj);
         items[i] = tmps;
         total_strlen += strlen(tmps);
@@ -488,14 +469,14 @@ char* comps_objlist_tostr_u(COMPS_Object* list) {
     ret[0]=0;
     strcat(ret, "[");
     //total2 += strlen("[");
-    for (i = 0; i < (int)(((COMPS_ObjList*)list)->len-1); i++) {
-        strcat(ret, items[i]);
-        //total2 += strlen(items[i]);
-        strcat(ret, ", ");
-        //total2 += strlen(", ");
-        free(items[i]);
-    }
     if (((COMPS_ObjList*)list)->len) {
+        for (i = 0; i < (int)(((COMPS_ObjList*)list)->len-1); i++) {
+            strcat(ret, items[i]);
+            //total2 += strlen(items[i]);
+            strcat(ret, ", ");
+            //total2 += strlen(", ");
+            free(items[i]);
+        }
         strcat(ret, items[i]);
         //total2 += strlen(items[i]);
         free(items[i]);
diff --git a/libcomps/src/comps_parse.c b/libcomps/src/comps_parse.c
index 902e663..bd11372 100644
--- a/libcomps/src/comps_parse.c
+++ b/libcomps/src/comps_parse.c
@@ -166,14 +166,12 @@ int comps_parse_validate_dtd(char *filename, char *dtd_file) {
     if (!ret) {
         err = xmlGetLastError();
         printf("%s\n", err->message);
+        ret = -err->code;
     }
     xmlFreeDoc(fptr);
     xmlFreeDtd(dtd_ptr);
     xmlFreeValidCtxt(vctxt);
-    if (!ret)
-        return -err->code;
-    else
-        return ret;
+    return ret;
 }
 
 void __comps_after_parse(COMPS_Parsed *parsed) {
diff --git a/libcomps/src/comps_set.c b/libcomps/src/comps_set.c
index 45dbc75..e3eecfa 100644
--- a/libcomps/src/comps_set.c
+++ b/libcomps/src/comps_set.c
@@ -108,10 +108,7 @@ char comps_set_add(COMPS_Set * set, void *item) {
             return 0;
         }
     }
-    if (set->data_constructor)
-        comps_hslist_append(set->data, item, 1);
-    else
-        comps_hslist_append(set->data, item, 1);
+    comps_hslist_append(set->data, item, 1);
     return 1;
 }
 
diff --git a/libcomps/src/python/src/pycomps_sequence.c b/libcomps/src/python/src/pycomps_sequence.c
index 41e8889..166ae4c 100644
--- a/libcomps/src/python/src/pycomps_sequence.c
+++ b/libcomps/src/python/src/pycomps_sequence.c
@@ -257,13 +257,17 @@ PyObject* list_get_slice(PyObject *self, PyObject *key) {
 
     clen = 0;
     it = ((PyCOMPS_Sequence*)self)->list->first;
+    if (!it)
+        return (PyObject*)result;
     for (i=0 ; i<istart; it=it->next, i++);
     while (clen != ilen) {
         comps_objlist_append(result->list, it->comps_obj);
         clen+=1;
-        for (i=0 ; i<istep && it != NULL; it=it->next,  i++);
-        if (!it) it = ((PyCOMPS_Sequence*)self)->list->first;
-        for (; i<istep; it=it->next, i++);
+        i = 0;
+        do {
+            for (; i<istep && it != NULL; it=it->next, i++);
+            if (!it) it = ((PyCOMPS_Sequence*)self)->list->first;
+        } while (i < istep);
     }
     return (PyObject*)result;
 }
diff --git a/libcomps/tests/check_utils.c b/libcomps/tests/check_utils.c
index d032eaa..2b69bd8 100644
--- a/libcomps/tests/check_utils.c
+++ b/libcomps/tests/check_utils.c
@@ -108,9 +108,9 @@ void print_category(COMPS_Object *obj) {
     disp_ord = (prop)?((COMPS_Num*)prop)->val:0;
     COMPS_OBJECT_DESTROY(prop);
     
-    printf("id: %s\n", id);
-    printf("name: %s\n", name);
-    printf("desc: %s\n", desc);
+    printf("id: %s\n", id ? id : "(null)");
+    printf("name: %s\n", name ? name : "(null)");
+    printf("desc: %s\n", desc ? desc : "(null)");
     printf("display_order: %d\n", disp_ord);
     free(id);
     free(name);

From c9b3502e47ee19b7773de5b23b78f8ea7365c26b Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 09:21:00 +0100
Subject: [PATCH 5/6] Fix: __comps_xml_prop() return 1/0 instead of random
 value

Returns 1 in success 0 otherwise.
Random value was returned before.
---
 libcomps/src/comps_utils.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libcomps/src/comps_utils.c b/libcomps/src/comps_utils.c
index 1f8378b..36583aa 100644
--- a/libcomps/src/comps_utils.c
+++ b/libcomps/src/comps_utils.c
@@ -38,10 +38,10 @@ inline char __comps_strcmp(void *s1, void *s2) {
 
 inline int __comps_xml_prop(char *key, char *val,
                              xmlTextWriterPtr writer) {
-    int retc, x;
-    retc = (x=xmlTextWriterStartElement(writer, BAD_CAST key)<0)?x:retc;
-    retc = (x=xmlTextWriterWriteString(writer, BAD_CAST val)<0)?x:retc;
-    retc = (x=xmlTextWriterEndElement(writer)<0)?x:retc;
+    int retc;
+    retc = xmlTextWriterStartElement(writer, BAD_CAST key) >= 0 ? 1 : 0;
+    retc &= xmlTextWriterWriteString(writer, BAD_CAST val) >= 0 ? 1 : 0;
+    retc &= xmlTextWriterEndElement(writer) >= 0 ? 1 : 0;
     return retc;
 }
 

From 3732d8825d839ea131f7241b88f7a21f73c0b773 Mon Sep 17 00:00:00 2001
From: Jaroslav Rohel <jrohel@redhat.com>
Date: Wed, 28 Nov 2018 09:21:47 +0100
Subject: [PATCH 6/6] Remove unused code

---
 libcomps/src/python/src/pycomps.c | 3 ---
 libcomps/tests/check_parse.c      | 8 --------
 2 files changed, 11 deletions(-)

diff --git a/libcomps/src/python/src/pycomps.c b/libcomps/src/python/src/pycomps.c
index 67f6c88..b34685c 100644
--- a/libcomps/src/python/src/pycomps.c
+++ b/libcomps/src/python/src/pycomps.c
@@ -108,9 +108,6 @@ char __pycomps_dict_to_def_opts(PyObject* pobj, void *cobj) {
         for (x = 0; keys1[x] != NULL; x++) {
             val = PyDict_GetItemString(pobj, keys1[x]);
             if (val) {
-                if (PyINT_CHECK(val)) {
-                    tmp = PyINT_ASLONG(val);
-                }
                 tmp = PyLong_AsLong(val);
                 if (tmp == COMPS_PACKAGE_DEFAULT ||
                     tmp == COMPS_PACKAGE_OPTIONAL ||
diff --git a/libcomps/tests/check_parse.c b/libcomps/tests/check_parse.c
index fcc0a66..b21ba49 100644
--- a/libcomps/tests/check_parse.c
+++ b/libcomps/tests/check_parse.c
@@ -64,11 +64,6 @@ START_TEST(test_comps_parse1)
     const char* cats_names[] = {"category 1", "category 2"};
     const int cats_gids[] = {1, 2};
 
-    const char* envs_ids[] = {"env 1"};
-    const char* envs_names[] = {"environment 1"};
-    const char* env_gids[][3] = {{"a", "b", "c"}};
-    const char* env_opids[][2] = {{" option 1 ", " option 2 "}};
-
     int ret;
     COMPS_Object *tmpobj, *tmpobj2;
     char *tmpstr;
@@ -305,13 +300,11 @@ START_TEST(test_comps_parse3)
 {
     FILE *fp;
     //char *err_log,
-    char *tmp_ch;
     COMPS_Parsed *parsed;
     COMPS_ObjListIt *it;
     int i;
     COMPS_ObjList *tmplist;
     COMPS_LogEntry* known_errors[3];
-    char *str;
     COMPS_Object *tmpobj;
 
     fprintf(stderr, "## Running test_parse3\n\n");
@@ -525,7 +518,6 @@ START_TEST(test_arch)
     COMPS_DocGroup *g;
     COMPS_DocCategory *c;
     COMPS_DocEnv *e;
-    COMPS_DocGroupPackage *p;
     COMPS_Str *str;
     int x;
     COMPS_ObjListIt *it;