Jump to content
43oh

passing strings around


Recommended Posts

I have a question about passing strings around with pointers.   I'm writing some code to make processing serial events simpler by implementing a rotating command stack.   The stack itself looks like this:

typedef struct
{
	char buffer[STACK_LEN][BUFFER_LEN];	//buffer data store.
	uint8_t status[STACK_LEN];              // 0x1 = process status; 0x2 = overflow status
        uint8_t buffer_index[STACK_LEN];        //which element of the buffer to write to
        uint8_t stack_index;			//which buffer are we writing to
}command_stack;

It's just a 2D char array with indices to track which buffer is 'active', where to write within that buffer, and some status flags to mark the buffer ready to process, overflowed, etc.

 

 

Writing strings to the stacks work great, and I can watch it all work in the debugger.  However, pulling stuff off the stack isn't working.  Here is some sample code that should initialize the stack, write "TEST" to the first buffer, then read it back off, but it doesn't work.  I've tried replacing 'char string' with 'char string[8]' (and changing references of &string to string), but no dice.  I spent a while playing with which how the pointers are setup with mixed results, so figured I would try and get an official answer.

command_stack test_stack;    	
buffer_init(&test_stack);

write_string(&test_stack, "TEST");

char string;
read_buffer(&test_stack, &string, 0);

And here is a subset of the functions (the ones used for the sample code anyway).

#include "msp430g2553.h"
#include "stack.h"
#include "stdint.h"
#include "string.h"

const uint8_t READY = 1;
const uint8_t OVERFLOW = 2;

void buffer_init(command_stack* stack)
{
	memset(stack, 0, sizeof(command_stack));
}


/*Functions to manipulate the state of the stack*/

// Write a single character to the active buffer element
void write_char(command_stack* stack, char c)
{
	//Find the stack index and buffer index
	uint8_t si = stack->stack_index;
	uint8_t bi = stack->buffer_index[si];

	//if the buffer index is the same as the buffer length
	//we are overflowing the buffer, so handle it
	if (bi == BUFFER_LEN )
	{
		stack->status[stack->stack_index] |= OVERFLOW;
	}else{
		stack->buffer[si][bi] = c;	   //store the character
		stack->buffer_index[si]++;         //advance the pointer
	}
}


//Write an entire string into the buffer.  Return the number of chars written
uint8_t write_string(command_stack* stack, char* string)
{
	uint8_t old_idx = stack->buffer_index[stack->stack_index];

	//write each character to the buffer
	while(*string) write_char(stack, *string++);

	//if there is overflow, the buffer stops writing, so determine the
	//difference in the buffer pointers.
	return stack->buffer_index[stack->stack_index] - old_idx;

}



/* Functions to read the buffers */

//Read the string stored in buffer <buffer>.  return characters read

uint8_t read_buffer(command_stack* stack, char* string, uint8_t buffer)
{

	int i = 0;

	//buffer_index -1 marks the end of the data that was last written to
	//the buffer, so only read up to that point
	for (i = 0; i < stack->buffer_index[stack->stack_index]-1; i++)
	{
             string[i] = stack->buffer[stack->stack_index][i];
	}
	return i;
}



Link to post
Share on other sites

Nothing obvious jumps out at me except-

 

When reading the buffer, it might be helpful to add a \0 character to the end of the string after the for() loop so whichever printer/writer function you're using doesn't keep dumping memory contents past that point, e.g.-

 

    for (i = 0; i < stack->buffer_index[stack->stack_index]-1; i++)
    {
string[i] = stack->buffer[stack->stack_index][i];
    }

   string = '\0';
    return i;

 

Also, when you say reading back "doesn't work", can you elaborate in more detail on the actual behavior occurring?

Link to post
Share on other sites

I feel a little better known I didn't royal bork something up.

 

string isn't altered...it just stays at 0x0;

 

Here is a screenshot of the debugger showing the debug code I'm running next to the stack

 

post-28692-0-23244300-1372610851_thumb.png

 

 

edit:

 

I found one bug in read_buffer and found where I think the problem is occurring.  The new function is:

uint8_t read_buffer(command_stack* stack, char* string, uint8_t buffer)
{

	int i = 0;

	//buffer_index -1 marks the end of the data that was last written to
	//the buffer, so only read up to that point
	for (i = 0; i < stack->buffer_index[buffer]; i++)
	{
           string[i] = stack->buffer[buffer][i];
	}
	string[i] = '\0';
	return i;
}

When I step through this function, during the for loop, 'string' appears to be set correctly until the return.  Although the variable dump shows:

 

Name        Type                         Value                             Location

string         unsigned char *        0x03D3 "TEST" (Hex)    Register R13
*(string)      unsigned char          0x54 (Hex)                     0x03D3
 
does this indicate I'm not properly dereferencing the pointer?
Link to post
Share on other sites

Looks right...

string is a pointer (variable of type "unsigned char *"), therefore a 16-bit unsigned integer referring to the memory address where the string begins, 0x03D3 in this case.  All pointers will be of that type regardless of the flavor -- void *, int *, command_stack *, etc. are all just canonical placeholders for a 16-bit (on MSP430, without the MSP430X 20-bit extensions for devices with >64KB flash) integer containing a simple memory location.

 

*(string) is the dereferenced contents of the first byte contained within string's memory address, which is 0x54 -- capital letter "T" in the ASCII character set.

Link to post
Share on other sites

Not sure if that's the real issue, but your test code declares string as a single character. Also the parameter buffer, presumably indicating the length of the receiving string-buffer, is ignored in your read function.

 

Try

 

char string[BUFFER_LEN];

 

Also the for loop in the read function uses an odd end condition. I'd check I against BUFFER_LEN too. i.e.

 

for (i = 0; i < BUFFER_LEN; i++) 

 

Or, respecting the target buffer length

 

for (i = 0; i < BUFFER_LEN && i < buffer; i++)
Link to post
Share on other sites

 

Not sure if that's the real issue, but your test code declares string as a single character. Also the parameter buffer, presumably indicating the length of the receiving string-buffer, is ignored in your read function.

 

Try

char string[BUFFER_LEN];

Also the for loop in the read function uses an odd end condition. I'd check I against BUFFER_LEN too. i.e.

for (i = 0; i < BUFFER_LEN; i++) 

Or, respecting the target buffer length

for (i = 0; i < BUFFER_LEN && i < buffer; i++)

No I think the end condition is correct; each command_stack buffer entry has a variable (stack->buffer_index[n]) indicating the # of bytes in that buffer, so read_buffer() really shouldn't read any more than that since the contents of the command_stack's buffer[n] will be undefined beyond that index.

 

The string does definitely need to be a char array, and it should be BUFFER_LEN+1 to account for the \0 terminator at the end.

Link to post
Share on other sites

That seems to do it.  I swear I tried that and it didn't work...thanks!

 

I need to use that as the limit on my for loop.  The structure is a stack of STACK_LEN buffers, each which are BUFFER_LEN long.  'buffer' is which buffer (the place in the stack) to work with.  Whenever a character is pushed to a buffer, stack->buffer_index[buffer] is advanced one, so it keeps track of the end of the buffer.  It's a ring buffer, so when I reuse a buffer, I don't clear it...I just overwrite, so using stack->buffer_index[buffer] as my upper limit ensures that I only pull off values that have been recently written and ignore old data.  Adding i < BUFFER_LEN isn't a bad idea, although the code responsible for advancing the buffer_index checks for overflow and shouldn't allow that to happen.

Link to post
Share on other sites

Yeah I guess when it comes to using read_buffer(), remember that is doing a copy so the destination variable needs to be a buffer of appropriate size too... char array of BUFFER_LEN+1

 

The alternate way to roll would be to obtain a direct reference to the command_stack buffer[n] array.  You could use a char * string; for that... a char pointer named string, then a buffer_ref() call or something returns a char * pointer to the stack->buffer[n] position.  Problem with that is using it; your write_char does not maintain a '\0' at the end of the buffer, so you'd have to implement that and make sure the struct declares it buffer[sTACK_LEN][bUFFER_LEN+1] ... It would save you from having to copy the buffer contents every time you access it, although it does remove your ability to modify that buffer if you ever intend to read it again later unmodified.

Link to post
Share on other sites

I kind of like that idea.  It wouldn't be difficult to make the code add a \0 at the end on write.  Do you think it would hurt performance, if instead of tracking the end of the string, I just used strcat to tack on extra chars?  I could 'reset' the buffer by writing a \0 to the first element.  That would allow direct access to the stack without requiring dedicated read/write functions.

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...