tags:

views:

87

answers:

4

I am trying to write a function to get a string from uart. Its for an embedded system so I don't want to use malloc.

The pointer that is passed to the getstring function seems to point to garbage after the gets_e_uart1() is called. I don't use pointers too often so I'm sure it is something really stupid and trivial that Im doing wrong.

Regards, V

int main()
    {   
        char *ptr  = 0;
        while(1)
        {

        gets_e_uart1(ptr, 100);
        puts_uart1(ptr);

    }
        return 0;
}*end main*/ 

//-------------------------------------------------------------------------
//gets a string and echos it
//returns 0 if there is no error

char getstring_e_uart1(char *stringPtr_, const int SIZE_)

{

char buffer_[SIZE_];
stringPtr_ = buffer_;
int start_ = 0, end_ = SIZE_ - 1;
char errorflag = 0;

/*keep geting chars until newline char recieved*/
while((buffer_[start_++] = getchar_uart1())!= 0x0D)
{       
    putchar_uart1(buffer_[start_]);//echo it

    /*check for end of buffer 
     wraparound if neccesary*/
    if(start_ == end_)
    {
        start_ = 0;
        errorflag = 1;
    }
}
putchar_uart1('\n');
putchar_uart1('\r');

/*check for end of buffer 
 wraparound if neccesary*/
if(start_ == end_)
{
    buffer_[0] = '\0';
    errorflag = 1;
}
else
{
    buffer_[start_++] = '\0';
}
return errorflag;
}

Update:

I decided to go with approach of passing a pointer an array to the function. This works nicely, thanks to everyone for the informative answers.

Updated Code:

 //-------------------------------------------------------------------------
 //argument 1 should be a pointer to an array, 
 //and the second argument should be the size of the array
 //gets a string and echos it
 //returns 0 if there is no error
char getstring_e_uart1(char *stringPtr_, const int SIZE_)
{

    char *startPtr_ = stringPtr_;
    char *endPtr_  = startPtr_ + (SIZE_ - 1);
    char errorflag = 0;

    /*keep geting chars until newline char recieved*/
    while((*stringPtr_ = getchar_uart1())!= 0x0D)
    {       
        putchar_uart1(*stringPtr_);//echo it
        stringPtr_++;

        /*check for end of buffer 
         wraparound if neccesary*/
        if(stringPtr_ == endPtr_)
        {
            stringPtr_ = startPtr_;
            errorflag = 1;
        }
    }
    putchar_uart1('\n');
    putchar_uart1('\r');

    /*check for end of buffer 
     wraparound if neccesary*/
    if(stringPtr_ == endPtr_)
    {
        stringPtr_ = startPtr_;
        *stringPtr_ = '\0';
        errorflag = 1;
    }
    else
    {
        *stringPtr_ = '\0';
    }
    return errorflag;
}
+1  A: 

In addition to Brian's answer I think you also have a problem here:

char getstring_e_uart1(char *stringPtr_, const int SIZE_)   
{  
char buffer_[SIZE_]; 
stringPtr_ = buffer_; 

after the function returns buffer is no longer valid but stringPtr_ would still point to it. You could make buffer_ static or allocate it globally

Jackson
Hmm I knew it was something stupid... I should of spotted that! Id rather not use global/static, Iv previously implemented a solution like that, I was looking for something a little more elegant. Can you suggest any thing else.Regards, V
volting
No, it doesn't. When the function returns, `ptr` in main is 0, exactly the same as when it started. You're right though, in that if he changes it to `**stringPtr_`, he'll have the problem where he's pointing to something on the stack that has gone out of scope.
Brian Roach
You've got to allocate the memory somehow. If it isn't a global array or a static array then you'll have to malloc it or allocate it in main and pass it into the function.
Jackson
+2  A: 

Hint: ptr is pointing to garbage before gets_e_uart1() is called.

You need to pass a pointer to your pointer to gets_e_uart1()

EDIT: Except ... if you're trying to have ptr in main() point to the buffer_ in your function ... you've got another problem. buffer_ is on the stack and is out of scope as soon as the function returns. You would need to malloc() that memory in your function.

char getstring_e_uart1(char **stringPtr_, const int SIZE_)
{

    char *buffer_ = (char*) malloc(SIZE_ * sizeof(char));
    *stringPtr_ = buffer_;

(Editing again because I wasn't paying attention to your main loop, sorry)

Then you're going to have to free it after calling puts_uart1(ptr) which means you mallocing and freeing constantly in that loop. It seems you have a fixed size for that buffer, why not just create it in main() and pass it into both functions?

Brian Roach
So if I create an array of a certain size in main and pass the array and the size of the array to the function, that should work right?(like I said I dont want to use malloc, so looking for other solutions)
volting
Pass a pointer to it, but yes. Then you'd just operate directly on the buffer you created in main() rather than creating one in getstring_e_uart1()
Brian Roach
Sorry thats what I meant as in a pointer to the first element. Im doing that now. I will post the results shortly. Thanks
volting
+1  A: 

You need to construct the buffer prior to invoking getstring_e_uart1(). The code as written will cause the buffer_ variable to be destroyed when the function exits.

I think you want something more like:

char buffer_[SIZE_];
while(1) {
  char* ptr = buffer_;
  gets_e_uart1(ptr, sizeof(buffer_));
  puts_uart1(ptr);
}

Note: I've done no verification regarding the reuse of the variable buffer_.

Henk
A: 

You don't need to make the buffer static or global - you just have to have the caller allocate it. The changes are trivial:

int main()
{   
    while(1)
    {
        char buffer[100] = { 0 };

        gets_e_uart1(buffer, sizeof buffer);
        puts_uart1(buffer);

    }
    return 0;
}

//-------------------------------------------------------------------------
//gets a string and echos it
//returns 0 if there is no error

char getstring_e_uart1(char *buffer_, const int SIZE_)
{
    int start_ = 0, end_ = SIZE_ - 1;
    char errorflag = 0;

    /* ... */
caf