name/value function

[beakie]

can anyone please tell me if the following looks right? its supposed to be a function to add a name / value pair to two arrays.

the arrays are defined as:

int HTTPHeaderFieldCount;
char** HTTPHeaderFieldKeys;
char** HTTPHeaderFieldValues;

they are initialised as:

HTTPHeaderFieldCount = 0;
HTTPHeaderFieldKeys = (char**)malloc(0);
HTTPHeaderFieldValues = (char**)malloc(0);

here is the function

  void AddNameValue(char* Key, char* Value, char*** KeyArray, char*** ValueArray, int* ArrayLength)
  {
    char** tmpKeys;
    char** tmpValues;
    tmpKeys = (char**)malloc((*ArrayLength + 1) * sizeof(char*));
    tmpValues = (char**)malloc((*ArrayLength + 1) * sizeof(char*));

    for (int i = 0; i < *ArrayLength; i++)
    {
      tmpKeys[i] = *KeyArray[i];
      tmpValues[i] = *ValueArray[i];
    }

    tmpKeys[*ArrayLength] = (char*)malloc(strlen(Key) * sizeof(char));
    tmpValues[*ArrayLength] = (char*)malloc(strlen(Value) * sizeof(char));
    tmpKeys[*ArrayLength] = Key;
    tmpValues[*ArrayLength] = Value;

    free(*KeyArray);
    free(*ValueArray);

    *KeyArray = tmpKeys;
    *ValueArray = tmpValues;

    *ArrayLength = *ArrayLength + 1;
  }

called with the following:

 AddNameValue(cKey, cValue, &HTTPHeaderFieldKeys, &HTTPHeaderFieldValues, &HTTPHeaderFieldCount);

be nice. cheers :)

[beakie]

Any thoughts?

[kbw]

From your for loop syntax, it's clear you're using C++ and not C. That being the case, I'm not clear why you're using malloc/free and managing memory directly yourself. The C++ Standard Library has very suitable improvements in this area.

Having said that, your AddPair algorithm is something like:
1. Ensure the arrays are large enough for the new pair
2. Append the new pair

#include <malloc.h>
#include <string.h>

int HTTPHeaderFieldCount = 0;
char** HTTPHeaderFieldKeys = (char**)malloc(0);
char** HTTPHeaderFieldValues = (char**)malloc(0);

bool AddNameValue(char* Key, char* Value, char*** KeyArray, char*** ValueArray, int* ArrayLength)
{
	// Grow array
	char** keys = (char**)realloc(*KeyArray, *ArrayLength + 1);
	char** values = (char**)realloc(*ValueArray, *ArrayLength + 1);
	if (keys)
		*KeyArray = keys;
	if (values)
		*ValueArray = values;
	if (!keys || !values)
		return false;	// out of memory

	// Append new values
	(*KeyArray)[*ArrayLength] = strdup(Key);
	(*ValueArray)[*ArrayLength] = strdup(Value);
	++(*ArrayLength);
	return true;
}

int main()
{
	AddNameValue(
		"Name", "Ragnampiza",
		&HTTPHeaderFieldKeys,
		&HTTPHeaderFieldValues,
		&HTTPHeaderFieldCount);
	return 0;
}

A C++ implementation:

#include <string>
#include <map>

typedef std::map<std::string, std::string> MapType;
MapType dict;

int main()
{
	dict["Name"] = "Ragnampiza";
	return 0;
}

[beakie]

The reason I used malloc/free is because I was getting memory issues...

Why don't I need this in c++? I am writing this on VC++ but it will eventually be used on an arduino...

[beakie]

I forgot to say thanks for your reply. Its exactly what I wanted :)

[beakie]

Is there any chance you could show me what this function would be WITHOUT the use of malloc/free? (C++ style)

Cheers

[kbw]

Do you mean using different underlying data structures? Did you look at the C++ implementation I posted above?

[beakie]

I did. Thank you.

When I try and compile this using the arduino IDE it complains about map... which is why I wanted to write my own.


I am trying to get my head around char arrays and 2 dimensional arrays in c++... so if the code I wrote can be rewritten WIHTOUT the malloc lines, how would I do that, please? If you have time?

Thanks for your time on this :)

[kbw]

the arduino IDE it complains about map... which is why I wanted to write my own.

Don't do that. You need to fix the environment first. std::map is part of the C++ standard library. It really should be available. Does Arduino explicitly disallow STL?

You can't manage a structure that grows dynamically without using the heap. You either need to use malloc/free or new/delete.

An alternative is to use a fixed length structure that has a fixed upper limit.

Is the malloc/free restriction related to the embedded environment?

[beakie]

This is what arduino sees as map

http://www.arduino.cc/en/Reference/map

Which isnt the same thing, right?

[kbw]

Mmm, Arduino looks like a C environment. I can't find any spec on the language, just examples.

Here's a fixed array C implementation of your dictionary.

#include <string.h>

#define DICT_MAXELEMENTS 20
#define DICT_MAXKEYLEN 10
#define DICT_MAXVALUELEN 15

#define TRUE 1
#define FALSE 0
typedef int BOOL;

typedef struct DictEntry
{
	char key[DICT_MAXKEYLEN];
	char value[DICT_MAXVALUELEN];
} DictEntry;

static DictEntry dict[DICT_MAXELEMENTS];
static unsigned dict_size;

BOOL DictIsFull()
{
	return dict_size == DICT_MAXELEMENTS;
}

BOOL DictAddEntry(const char *key, const char *value)
{
	if (dict_size < DICT_MAXELEMENTS)
	{
		strncpy(dict[dict_size].key, key, DICT_MAXKEYLEN);
		strncpy(dict[dict_size].value, value, DICT_MAXVALUELEN);
		++dict_size;
		return TRUE;
	}

	return FALSE;
}

const char *DictLookupEntry(const char *key)
{
	unsigned i;
	for (i = 0; i != dict_size; ++i)
	{
		if (strncmp(dict[i].key, key, DICT_MAXKEYLEN) == 0)
			return dict[i].value;
	}

	return NULL;
}

[beakie]

Thanks for that. I will have a play with this!

Just out of curiosity... I am trying to understand why I am getting a null exception (in the function in my original post) when ArrayLength is 2. It seems to be something to do with their being multiple previous values to move to the new array.

Looking at the original function I posted... I get an error on the loop that populates previous key/values.

for (int i = 0; i < *ArrayLength; i++)
{
  tmpKeys[i] = *KeyArray[i];
  tmpValues[i] = *ValueArray[i];  //  <<<--- error occurs here first
}

I get the null error "object reference not set to an instance of an object". The weird thing is, when I step it through I don't see where the null is coming from.

Both tmpValues[i] and *ValueArray[i] have a value so wheres the null? I am a VB programmer so if there is something wrong that is c++ specific please let me know so I can try and understand it.


Thanks :)

[kbw]

You really shouldn't grow a malloc'd block of memory in that way, that's what realloc's for. But you have to be very careful with realloc.

I didn't really look at your code in detail, I just look at it and thought "Yuk". Now that I'm looking at it:

    tmpKeys = (char**)malloc((*ArrayLength + 1) * sizeof(char*));
    tmpValues = (char**)malloc((*ArrayLength + 1) * sizeof(char*));

should be:

    tmpKeys = (char**)malloc(((*ArrayLength) + 1) * sizeof(char*));
    tmpValues = (char**)malloc(((*ArrayLength) + 1) * sizeof(char*));

This:

    tmpKeys[*ArrayLength] = (char*)malloc(strlen(Key) * sizeof(char));
    tmpValues[*ArrayLength] = (char*)malloc(strlen(Value) * sizeof(char));
    tmpKeys[*ArrayLength] = Key;
    tmpValues[*ArrayLength] = Value;

should be:

    tmpKeys[*ArrayLength] = (char*)malloc((strlen(Key) + 1) * sizeof(char));
    tmpValues[*ArrayLength] = (char*)malloc((strlen(Value) + 1) * sizeof(char));
    strcpy(tmpKeys[*ArrayLength], Key);
    strcpy(tmpValues[*ArrayLength]. Value);

or simply:

    tmpKeys[*ArrayLength] = strdup(Key);
    tmpValues[*ArrayLength] = strdup(Value);

[beakie]

Once again... thank you, Sir.

I will have a play. I appreciate your assistance with this.

[beakie]

Ummmmm I still get the null error :(

[beakie]

Rewrote with your suggestions and it worked!

  void AddNameValue(char* Key, char* Value, char*** KeyArray, char*** ValueArray, int* ArrayLength)
  {
	char** tmpKeys = (char**)realloc(*KeyArray, (*ArrayLength + 1) * sizeof(char*));
	char** tmpValues = (char**)realloc(*ValueArray, (*ArrayLength + 1) * sizeof(char*));

	if (tmpKeys)
		*KeyArray = tmpKeys;
	if (tmpValues)
		*ValueArray = tmpValues;
//	if (!keys || !values)
//		return false;	// out of memory

	(*KeyArray)[*ArrayLength] = strdup(Key);
	(*ValueArray)[*ArrayLength] = strdup(Value);
	++(*ArrayLength);
  }

I would still like to understand why I was getting a null error on a line with no obvious null involved... but at least it works now.... Thanks :)

Thanks for your help with this man.