simple pointer question

Forum 2005-2010 (read only) Software Syntax & Programs
1

I am having a problem getting valid boolean data from a pointer. If
I print out the dataBit variable in the readDataBit() function I get either a 1 or a 0 (great)... but if I print it out in the calling function I get a completely different (non-boolean) number.

What am I doing wrong?

boolean * readDataBit() {

boolean dataBit = false;
//
// logic sets boolean to true or false
//
Serial.print("bit: ");
Serial.println(*dataBit, DEC); // this prints 1 or 0

return &dataBit;
}

void processBit() {
boolean *dataBit = readDataBit();

Serial.print("bit: ");
Serial.println(*dataBit, DEC); // this prints 53 or 13569
}

2

I got it... I'm new to this (obviously). I simply needed to dynamically allocate memory to the variable i was returning:

boolean * readDataBit() {
 
  boolean *dataBit = (boolean*) malloc(1 * sizeof (boolean));
  ...
}

Thanks.

3

Oooh, yikes. That looks like a recipe for running out of memory pretty fast, because the dynamically allocated memory is never freed.

Why does readDataBit need to return a pointer? Could it not be rewritten as

boolean readDataBit()
{
  boolean databit;
  ...
  return databit;
}

?

Best,

Mikal

4

Thanks for the advice. I am freeing up the memory in the code that calls the readBitData(). I am returning a pointer because i needed some way to represent NULL data (e.g. the end of a bit stream). I wanted to use a boolean type to represent a bit rather than an integer because it seemed to more closely resemble the data I was looking for represent (a bit).

Does this seem kludgy to you? I read that anytime you are given a pointer to data from a function call that you should presume it needs to be freed... so the function depends on the user-programmer to free the data... Is this good practice?

Any advice on what is considered elegant and what is not is very welcome!

boolean *dataBit = readDataBit();
...
free(dataBit);
5

Mac, your reasoning and explanation are both very sound. You really want a data type that has three states, right, "on", "off", and "null".

I think your scheme probably works fine, although I think if I were implementing it, I don't think I'd use the dynamic allocation, obviously because there is always that slight danger of introducing bugs with mismatched malloc/free pairs, but also because it introduces a bit of inefficiency, both in terms of space and time. Every call to your readDataBit consumes three bytes: the 2-byte pointer, plus the 1-byte boolean value, so that's actually less efficient than if you had chosen int or byte as the return value in the first place. Also, you suffer a performance penalty, because every call to malloc and free adds an unnecessary waste of processing time.

Bravo for conceiving and acting upon the idea that a boolean "more closely resembles the data" you are trying to represent. This is a very good programming practice. In this case, though, boolean may not quite suffice, because of the need to also represent the "end" token. I think if I were building this, I'd probably write:

enum {BIT_ON, BIT_OFF, BIT_END};
byte readDataBit()
{
   ...
   return BIT_ON;
   ...
   return BIT_END;
}

// usage:
byte dataBit = readDataBit();
if (dataBit == BIT_END) // do something for end of stream

If you really want to use boolean, you could write readDataBit() to return a success/failure to indicate whether the end of the bit stream had been reached and modify dataBit as a reference parameter.

nice job

Mikal

6

Thanks mikalhart I really like your enum idea :slight_smile:

While I have your ear. I also would like to return an array of bytes from the method that calls readDataBit() (as that is ultimately what i am constructing out of the bits). It seems that there is no easy way to do this in C however so I have resorted to using a home spun linked list (which seems ugly).

How would you handle this?

7

In case it helps this is the struct i am using to represent a byte array:

typedef struct ByteArray {
  byte *data;
  int size;
} ByteArray;

This is the code I am using to cycle over and print out the data:

  ByteArray *byteArray = readByteArray();
  
  if(byteArray->size != 0) {    
    for(int i = 0; i < byteArray->size; i++) {
      Serial.print(byteArray->data[i], HEX);
      if(i < byteArray->size - 1) Serial.print("-");
    }
    Serial.println("");
  }

Which outputs strings in the following format (for a 5 byte array):

0-8B-8B-4C-1

Anyone want to chime in on the elegance/inelegance of this approach? Thank you very much in advance!!!

8

That's not too bad! Inside ByteArray how do you create the "data" buffer, and how do you know how big to make it? BTW, that is not a strictly speaking linked list.

One alternative that would circumvent having to do dynamic allocation would be to have the caller pass a pointer to the array and its size like this:

// caller
byte array[100];
readDataBytes(array, sizeof(array));

// implementation
void readDataBytes(byte *buf, int size)
{
...
}

Mikal

9

I use this call to dynamically increase the size of the array:

void add_byte(byte_array_s *byteArray, byte _byte) {
  byte *array = (byte*) realloc(byteArray->data, sizeof(byte) * byteArray->size + 1);
  byteArray->data = array;
  byteArray->size++;
  byteArray->data[byteArray->size - 1] = _byte;
}

As I cannot tell beforehand how much data there is to read I cannot pass in a statically sized array. Sometimes the data is 40 bits long and other times it is 26 bit or much larger or smaller.

I know that the struct I pasted isn't a linked list... I decided that I wanted to use a more traditional array instead so I changed it :slight_smile:

(Sorry I should also note that I chagned the name of the typedef from 'ByteArray' to 'byte_array_s' as it seemed more 'C'ish)

10

Also... can you tell me if it is valid ANSI C to include methods in your structs like so:

typedef struct byte_array {
  byte *data;
  int size;
  
  void add_byte(byte _byte) {
    byte *array = (byte*) realloc(data, sizeof(byte) * size + 1);
    data = array;
    size++;
    data[size - 1] = _byte;
  }

} byte_array_s;
11

Keep in mind that if you use realloc() in that way, you are limiting your already precious RAM supply. There would be no way, for example, to increase the size of your buffer from 512 to 513 bytes, because realloc requires (for a time anyway) both the old and new buffers to be simultaneously allocated.

Do you have any notion what the maximum possible size for the array is? If it's pretty large, I would recommend the static byte array that is passed in by the caller, or possibly even a bit array.

Adding a method to a struct is not ANSI C, nor C of any kind. That is a C++ technique. But there is no problem here; Arduino is C++ (almost) all the way. (An interesting bit of trivia is that the only difference between a struct and a C++ class is the default member visibility: public for struct and private for class.)

Mikal

12

Thank you very much for all the advise. I will revise the method :slight_smile: