| View previous topic :: View next topic |
| Author |
Message |
Huyvu90
Guest
|
| What's wrong with this parse function? |
 Posted: Sat May 06, 2006 5:31 am |
 |
|
values = "M 1 2 3 4 5 6 7 8 9"
The goal of parsevalues() is to split up the numbers in variable "values" and store them in an int array of 9 elements (parameters[]).
The function parsevalues() does somewhat work but the array doesn't seem to be updating as often. Are there any error in the code that I missed?
| Quote: | void parsevalues()
{
int i = 0;
int j = 0;
char singlevalue[3];
char term[3], *ptr;
strcpy(term," ");
ptr = strtok(values, term);
while(ptr!=0) {
strcpy(singlevalue,ptr);
if(i<=9)
{
parameters[i] = atoi(singlevalue);
i++;
}
ptr = strtok(0, term);
}
} |
|
|
 |
Mark
Joined: 07 Sep 2003
Posts: 2838
Location: Atlanta, GA
|
|
| Posted: Sat May 06, 2006 8:50 am |
|
|
| The first time through were will ptr point? Then you do a strcpy into singlevalue. You WILL overflow this. singlevalue should be at least the same size as values. |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Sat May 06, 2006 10:37 am |
|
|
| Mark wrote: | | The first time through were will ptr point? |
He did initialize pointer, that's allright.
- You are only interested in the numbers? Than add an extra call to strtok() in order to skip the first element in your string.
| Quote: | | and store them in an int array of 9 elements (parameters[]) |
Your counter i starts at 0, so use '<' instead of '<='.
- As Mark already pointed out the strcpy is wrong because of array overflowing. Strcpy continues copying until it reaches the null character at the end of the string while you want it to stop at the space character between the parameters.
Note: The example program for strtok() in the manual has the same error.
Possible solutions are:
1) Write your own copy routine that stops at the space character. This is the best solution.
2) A little bit quicker but less flexible: Use strncpy instead of strcpy. Strncpy will stop the copy at the maximum amount of characters you specify.
3) Or quick-and-dirty, don't copy at all using the knowledge that atoi() will stop at any non-digit character. | Code: | void parsevalues()
{
int i = 0;
char term[3], *ptr;
strcpy(term," ");
// Extra call to strtok in order to skip first parameter.
ptr = strtok(values, term);
ptr = strtok(0, term);
while (ptr!=0)
{
if (i<9) // Changed '<=' to '<'
{
parameters[i] = atoi(ptr); // atoi will stop at first non-digit character (space or terminating zero).
i++;
}
ptr = strtok(0, term);
}
}
|
Edit 1: Moved the note about the error in the CCS example to the correct paragraph.
Edit 2: Fixed a bug in skipping the first parameter (now pass 0 as parameter to strtok).
Last edited by ckielstra on Wed May 10, 2006 2:05 am; edited 2 times in total |
|
|
Mark
Joined: 07 Sep 2003
Posts: 2838
Location: Atlanta, GA
|
|
| Posted: Sun May 07, 2006 9:50 am |
|
|
| ckielstra wrote: | | Mark wrote: | | The first time through were will ptr point? |
He did initialize pointer, that's allright.
|
I did see that. It was a rhetorical question trying to get the poster to visualize where the pointer was pointer and see how long the string would be. Reading back over my post, I can see how unclear I was. |
|
|
HUyvu80
Guest
|
| didn't work |
| Posted: Tue May 09, 2006 11:21 pm |
|
|
| ckielstra wrote: | | Mark wrote: | | The first time through were will ptr point? |
He did initialize pointer, that's allright.
- You are only interested in the numbers? Than add an extra call to strtok() in order to skip the first element in your string.
| Quote: | | and store them in an int array of 9 elements (parameters[]) |
Your counter i starts at 0, so use '<' instead of '<='.
- As Mark already pointed out the strcpy is wrong because of array overflowing. Strcpy continues copying until it reaches the null character at the end of the string while you want it to stop at the space character between the parameters.
Note: The example program for strtok() in the manual has the same error.
Possible solutions are:
1) Write your own copy routine that stops at the space character. This is the best solution.
2) A little bit quicker but less flexible: Use strncpy instead of strcpy. Strncpy will stop the copy at the maximum amount of characters you specify.
3) Or quick-and-dirty, don't copy at all using the knowledge that atoi() will stop at any non-digit character. | Code: | void parsevalues()
{
int i = 0;
char term[3], *ptr;
strcpy(term," ");
// Extra call to strtok in order to skip first parameter.
ptr = strtok(values, term);
ptr = strtok(values, term);
while (ptr!=0)
{
if (i<9) // Changed '<=' to '<'
{
parameters[i] = atoi(ptr); // atoi will stop at first non-digit character (space or terminating zero).
i++;
}
ptr = strtok(0, term);
}
}
|
Edit: Moved the note about the error in the CCS example to the correct paragraph. |
The code didn't work. I looked up atoi and it has the following
"Parameters:
string is a pointer to a null terminated string of characters."
atoi does not stop whitespace or 0 like u said but a null character like strcpy.
Is there anyway except writing my own parse function? |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Wed May 10, 2006 2:06 am |
|
|
| Quote: | The code didn't work. I looked up atoi and it has the following
"Parameters:
string is a pointer to a null terminated string of characters."
atoi does not stop whitespace or 0 like u said but a null character like strcpy. |
atoi stops at a null character like strcpy but also at the first whitespace. Check the code for atoi in the file stdlib.h, here you see the following loop: | Code: | while (c >= '0' && c <= '9')
{
result = 10*result + (c - '0');
c = s[index++];
} |
atoi() will stop parsing at any character out of the range '0' - '9'.
I tested the function above in the MPLAB simulator and CCS version 3.226 and it works great. There was only an error in skipping the first parameter, see my edited code in the program above.
If it doesn't work for you then there must be an error somewhere else in your program. Post a complete compilable program and we will have a look. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
