Skip to content

Commit

Permalink
fix various memory problems
Browse files Browse the repository at this point in the history
  • Loading branch information
kodebach committed Oct 2, 2020
1 parent 1bf60ee commit d93027f
Show file tree
Hide file tree
Showing 15 changed files with 136 additions and 84 deletions.
2 changes: 1 addition & 1 deletion src/libs/ease/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ int elektraArrayIncName (Key * key)

kdb_long_long_t oldIndex = 0;
if (offsetIndex && elektraReadArrayNumber (baseName, &oldIndex) == -1) return -1;
kdb_long_long_t newIndex = offsetIndex ? oldIndex + 1 : 0; // we increment by one or use 0 if the name contains no index yet
kdb_long_long_t newIndex = offsetIndex != 0 ? oldIndex + 1 : 0; // we increment by one or use 0 if the name contains no index yet

char newName[ELEKTRA_MAX_ARRAY_SIZE];

Expand Down
17 changes: 10 additions & 7 deletions src/libs/elektra/key.c
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,13 @@ int keyCopy (Key * dest, const Key * source)
return -1;
}

static void keyClearNameValue (Key * key)
{
if (key->key && !test_bit (key->flags, KEY_FLAG_MMAP_KEY)) elektraFree (key->key);
if (key->ukey && !test_bit (key->flags, KEY_FLAG_MMAP_KEY)) elektraFree (key->ukey);
if (key->data.v && !test_bit (key->flags, KEY_FLAG_MMAP_DATA)) elektraFree (key->data.v);
}


/**
* A destructor for Key objects.
Expand Down Expand Up @@ -580,8 +587,6 @@ int keyCopy (Key * dest, const Key * source)
*/
int keyDel (Key * key)
{
int rc;

if (!key) return -1;

if (key->ksReference > 0)
Expand All @@ -591,7 +596,7 @@ int keyDel (Key * key)

int keyInMmap = test_bit (key->flags, KEY_FLAG_MMAP_STRUCT);

rc = keyClear (key);
keyClearNameValue (key);

ksDel (key->meta);

Expand All @@ -600,7 +605,7 @@ int keyDel (Key * key)
elektraFree (key);
}

return rc;
return 0;
}

/**
Expand Down Expand Up @@ -645,9 +650,7 @@ int keyClear (Key * key)

int keyStructInMmap = test_bit (key->flags, KEY_FLAG_MMAP_STRUCT);

if (key->key && !test_bit (key->flags, KEY_FLAG_MMAP_KEY)) elektraFree (key->key);
if (key->ukey && !test_bit (key->flags, KEY_FLAG_MMAP_KEY)) elektraFree (key->ukey);
if (key->data.v && !test_bit (key->flags, KEY_FLAG_MMAP_DATA)) elektraFree (key->data.v);
keyClearNameValue (key);

ksDel (key->meta);

Expand Down
4 changes: 2 additions & 2 deletions src/libs/elektra/keymeta.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,11 +506,11 @@ ssize_t keySetMeta (Key * key, const char * metaName, const char * newMetaString
}
}

if (newMetaString)
if (newMetaString != NULL)
{
/*Add the meta information to the key*/
metaStringDup = elektraStrNDup (newMetaString, metaStringSize);
if (!metaStringDup)
if (metaStringDup == NULL)
{
// TODO: actually we might already have changed
// the key
Expand Down
9 changes: 5 additions & 4 deletions src/libs/elektra/keyname.c
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ ssize_t keyGetName (const Key * key, char * returnedName, size_t maxSize)
return -1;
}

memcpy (returnedName, key->key, maxSize);
memcpy (returnedName, key->key, key->keySize);

return key->keySize;
}
Expand Down Expand Up @@ -1535,7 +1535,7 @@ ssize_t keyAddName (Key * key, const char * newName)
{
// skip leading slashes
++newName;
if (*newName == '.' && (*(newName + 1) == '/' || *(newName + 1) == '\0'))
if (*newName == '.' && *(newName + 1) == '/')
{
// also skip /./ parts
newName += 2;
Expand Down Expand Up @@ -1620,7 +1620,7 @@ static const char * elektraKeyFindBaseNamePtr (Key * key)
}

size_t backslashes = 0;
while (*(cur - backslashes - 1) == '\\')
while (cur - backslashes > start && *(cur - backslashes - 1) == '\\')
{
++backslashes;
}
Expand Down Expand Up @@ -1832,7 +1832,8 @@ ssize_t keySetNamespace (Key * key, elektraNamespace ns)
}

memcpy (key->key, newNamespace, newNamespaceLen);
key->keySize += newNamespaceLen - oldNamespaceLen;
key->keySize += newNamespaceLen;
key->keySize -= oldNamespaceLen;
key->key[key->keySize - 1] = '\0';

key->ukey[0] = ns;
Expand Down
2 changes: 1 addition & 1 deletion src/libs/elektra/keytest.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ int keyIsInactive (const Key * key)

const char * cur = uname + 1; // skip namespace

while (cur != NULL && cur <= uname + size)
while (cur != NULL && cur + 1 < uname + size)
{
++cur;
if (cur[0] == '.')
Expand Down
1 change: 1 addition & 0 deletions src/plugins/augeas/augeas.c
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ int elektraAugeasGet (Plugin * handle, KeySet * returned, Key * parentKey)

ret = foreachAugeasNode (augeasHandle, AUGEAS_TREE_ROOT, &convertToKey, conversionData);

keyDel (conversionData->parentKey);
elektraFree (conversionData);

if (ret < 0)
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/spec/spec.c
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,8 @@ static void copyMeta (Key * dest, Key * src)
keyCopyMeta (dest, src, name);
}
}

ksDel (metaKS);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/type/types.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,9 @@ bool elektraTypeNormalizeEnum (Plugin * handle ELEKTRA_UNUSED, Key * key)

keySetBaseName (valueKey, value);
Key * cur = ksLookup (validValues, valueKey, 0);
keyDel (valueKey);
if (cur == NULL)
{
keyDel (valueKey);
ksDel (validValues);
elektraFree (values);
return false;
Expand Down
148 changes: 93 additions & 55 deletions src/plugins/xmltool/kscompare.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,61 @@

#include "xmltool.h"

#include <stdlib.h>
#include <string.h>

static Key * commonParent (Key * firstKey, Key * secondKey, size_t maxSize)
{
// First we find the common prefix of the first two keys.
// NOTE: a common prefix is not necessarily a common parent
// e.g. system:/abc/d is a common prefix of system:/abc/de and system:/abc/df,
// but the common parent would be system:/abc

const char * firstName = keyName (firstKey);
const char * secondName = keyName (secondKey);

size_t commonLength = 0;
for (size_t i = 0; i < maxSize; ++i)
{
if (firstName[i] == '\0' || secondName[i] == '\0')
{
break;
}

if (firstName[i] != secondName[i])
{
break;
}

commonLength = i + 1;
}

if (commonLength == 0)
{
return NULL;
}

// We now extract the common prefix ...
char * commonPrefix = strndup (firstName, commonLength);

// ... and adjust it to a common parent.
Key * common = keyNew (commonPrefix, KEY_END);
if (commonPrefix[commonLength - 1] != '/')
{
keySetBaseName (common, NULL);
}

free (commonPrefix);

if ((size_t) keyGetNameSize (common) > maxSize)
{
keyDel (common);
return NULL;
}

return common;
}


/**
* @internal
Expand All @@ -19,7 +72,7 @@
* This is a c-helper function, you need not implement it in bindings.
*
* Given the @p ks KeySet, calculates the parent name for all the keys.
* So if @p ks contains this keys:
* So if @p ks contains these keys:
*
* @code
* system:/sw/xorg/Monitors/Monitor1/vrefresh
Expand All @@ -44,72 +97,57 @@
* @param returnedCommonParent a pre-allocated buffer that will receive the
* common parent, if found
* @param maxSize size of the pre-allocated @p returnedCommonParent buffer
* @return size in bytes of the parent name, or 0 if there is no common parent,
* or -1 to indicate an error, then @p errno must be checked.
* @return size in bytes of the parent name, or 0 if there is no common parent (with length <= maxSize)
*/
ssize_t ksGetCommonParentName (const KeySet * working, char * returnedCommonParent, size_t maxSize)
size_t ksGetCommonParentName (KeySet * working, char * returnedCommonParent, size_t maxSize)
{
size_t parentSize = 0;
Key * current = 0;
elektraCursor cinit;
KeySet * ks;
ssize_t sMaxSize;
if (maxSize > SSIZE_MAX) return 0;
if (ksGetSize (working) < 1) return 0;

if (maxSize > SSIZE_MAX) return -1;
sMaxSize = maxSize;
if (ksGetSize (working) == 1)
{
return keyGetName (ksAtCursor (working, 0), returnedCommonParent, maxSize);
}

cinit = ksGetCursor (working);
ks = (KeySet *) working;
// Get common parent of first two keys in the KeySet.

if (ksGetSize (ks) < 1) return 0;
Key * common = commonParent (ksAtCursor (working, 0), ksAtCursor (working, 1), maxSize);

ksRewind (ks);
current = ksNext (ks);
if (keyGetNameSize (current) > sMaxSize)
if (common == NULL)
{
/*errno=KDB_ERR_TRUNC;*/
returnedCommonParent[0] = 0;
return -1;
*returnedCommonParent = '\0';
return 0;
}

strcpy (returnedCommonParent, keyName (current));
parentSize = elektraStrLen (returnedCommonParent);
// We then check if all keys in the KeySet are below the parent we found.
KeySet * cut = ksCut (working, common);

while (*returnedCommonParent)
while (ksGetSize (working) != 0)
{
ksRewind (ks);
while ((current = ksNext (ks)) != 0)
{
/* Test if a key doesn't match */
if (memcmp (returnedCommonParent, keyName (current), parentSize - 1)) break;
}
if (current)
{
/* some key failed to be a child */
/* parent will be the parent of current parent... */
char * delim = 0;

// TODO: does not honor escaped characters
if ((delim = strrchr (returnedCommonParent, KDB_PATH_SEPARATOR)))
{
*delim = 0;
parentSize = elektraStrLen (returnedCommonParent);
}
else
{
*returnedCommonParent = 0;
parentSize = 0;
break; /* Better don't make comparison with parentSize-1 now */
}
}
else
// If not all keys match, we find the common prefix of common and the first non-matching key.
Key * nextKey = ksAtCursor (working, 0);

ksAppend (working, cut);
ksDel (cut);

Key * newCommon = commonParent (common, nextKey, maxSize);

keyDel (common);
common = newCommon;

if (common == NULL)
{
/* All keys matched (current==0) */
/* We have our common parent to return in commonParent */
ksSetCursor (ks, cinit);
return parentSize;
*returnedCommonParent = '\0';
return 0;
}

cut = ksCut (working, common);
}
ksSetCursor (ks, cinit);
return parentSize; /* if reached, will be zero */

ksAppend (working, cut);
ksDel (cut);

ssize_t ret = keyGetName (common, returnedCommonParent, maxSize);
keyDel (common);
return ret;
}
8 changes: 4 additions & 4 deletions src/plugins/xmltool/testmod_xmltool.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,13 @@ static void test_ksCommonParentName (void)
ksDel (ks);

ks = ksNew (10, keyNew ("system:/some/thing", 0), keyNew ("system:/other/thing", 0), KS_END);
succeed_if (ksGetCommonParentName (ks, ret, MAX_SIZE) == 8, "could find correct parentname");
succeed_if_same_string (ret, "system:");
succeed_if (ksGetCommonParentName (ks, ret, MAX_SIZE) == 9, "could find correct parentname");
succeed_if_same_string (ret, "system:/");
ksDel (ks);

ks = ksNew (10, keyNew ("system:/some/thing", 0), keyNew ("system:/something", 0), KS_END);
succeed_if (ksGetCommonParentName (ks, ret, MAX_SIZE) == 13, "could find correct parentname");
succeed_if_same_string (ret, "system:/some"); // TODO (kodebach): wrong?
succeed_if (ksGetCommonParentName (ks, ret, MAX_SIZE) == 9, "could find correct parentname");
succeed_if_same_string (ret, "system:/");
ksDel (ks);

ks = ksNew (10, keyNew ("system:/here/in/deep/goes/ok/thing", 0), keyNew ("system:/here/in/deep/goes/ok/other/thing", 0), KS_END);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/xmltool/xmltool.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <stdio.h>


ssize_t ksGetCommonParentName (const KeySet * ks, char * returnedCommonParent, size_t maxSize);
size_t ksGetCommonParentName (KeySet * ks, char * returnedCommonParent, size_t maxSize);
size_t elektraStrLen (const char * s);

int elektraXmltoolOpen (Plugin * handle, Key * errorKey);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/yajl/iterator.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ int elektraKeyNameReverseNext (keyNameReverseIterator * it)
}

size_t backslashes = 0;
while (*(real - backslashes - 1) == KDB_PATH_ESCAPE)
while (real - backslashes > it->rend && *(real - backslashes - 1) == KDB_PATH_ESCAPE)
{
++backslashes;
}
Expand Down
10 changes: 5 additions & 5 deletions src/plugins/yajl/name.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ ssize_t elektraKeyCountEqualLevel (const Key * cmp1, const Key * cmp2)
const char * next1 = strchr (cur1, '\0');
const char * next2 = strchr (cur2, '\0');

if (pcmp1 + size1 <= cur1 || pcmp2 + size2 <= cur2)
{
break;
}

++counter;

cur1 = next1 + 1;
cur2 = next2 + 1;

if (pcmp1 + size1 <= cur1 || pcmp2 + size2 <= cur2)
{
break;
}
}

return counter;
Expand Down
2 changes: 1 addition & 1 deletion tests/abi/testabi_ks.c
Original file line number Diff line number Diff line change
Expand Up @@ -1051,7 +1051,7 @@ static void test_ksLookup (void)
succeed_if (ksLookup (ks, k[j], 0) == k[j], "did not find key");
}
succeed_if (ksLookup (ks, k[23], 0) == 0, "found wrong key");
succeed_if (ksLookup (ks, k[24], 0) == 0, "found wrong key");
succeed_if (ksLookup (ks, k[26], 0) == 0, "found wrong key");
succeed_if (ksLookup (ks, k[28], 0) == k[6], "did not find key");
succeed_if (ksLookup (ks, k[32], 0) == k[6], "did not find key");
succeed_if (ksLookup (ks, k[33], 0) == 0, "found wrong key");
Expand Down

0 comments on commit d93027f

Please sign in to comment.