CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to CCS Technical Support

String parsing problem

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Zloi



Joined: 26 Oct 2012
Posts: 11
Location: Croatia

View user's profile Send private message

String parsing problem
PostPosted: Sat Nov 17, 2012 3:30 pm     Reply with quote

Hello, I have problem. I want to parse the string of variable length.
String is consisting of a command and the value and is null terminated.
I use the strstr() to search for command in received string,
When i find the command, then I convert the numbers that follow with
the help of strtoul() function, the converted number is stored in the
corresponding variable.

String example:
L1YE200 L2YE303 L3YE40 LGRN20

L1YE is command 200 is value
L2YE is command 300 is value..etc

I wrote the function but has a flaw, it works well for 1 or 2 commands sent to pic, but for more is faulty.

my code:
Code:

void MsgDecode (void){
   unsigned int16 TempNum;
   char *p;
   char *tmp;
   
   if(bit_test(FraimFlags,FrameDecode)){
      bit_clear(FraimFlags,FrameDecode);
      strcpy(COMMAND,COMMAND_Buf);
      clean_up_str(COMMAND);
      printf(bputc,COMMAND);
      
      strcpy(COMM,"L1YE");                        
      *p=strstr(COMMAND,COMM);
      if(*p!=0){

         TempNum=strtoul((*p+strlen(COMM)),&tmp,10);
         Left.Y1Time=TempNum;
         printf(bputc,"Left.Y1Time=%Lu ms\r\n",Left.Y1Time);
         *p=0;      
      }   

      strcpy(COMM,"L2YE");                        
      *p=strstr(COMMAND,COMM);
      if(*p!=0){
         TempNum=strtoul((*p+strlen(COMM)),&tmp,10);
         Left.Y2Time=TempNum;
         printf(bputc,"Left.Y2Time=%Lu ms\r\n",Left.Y2Time);
         *p=0;
      
      }   

      strcpy(COMM,"L3YE");                        
      *p=strstr(COMMAND,COMM);
      if(*p!=0){
         TempNum=strtoul((*p+strlen(COMM)),&tmp,10);
         Left.Y3Time=TempNum;
         printf(bputc,"Left.Y3Time=%Lu ms\r\n",Left.Y3Time);
         *p=0;   
      }   

      strcpy(COMM,"LGRN");                     
      *p=strstr(COMMAND,COMM);
      if(*p!=0){
         TempNum=strtoul((*p+strlen(COMM)),&tmp,10);
         Left.Reaction=TempNum;
         printf(bputc,"Left.Reaction=%Lu ms\r\n",Left.Reaction);
         *p=0;
      }   




   bit_clear(FraimFlags,FraimCheck);
   bit_clear(FraimFlags,FrameCopy);
   }
}



Terminal printout:
PC>MCU: L1YE200 L2YE300 L3YE40 LGRN20
MCU>PC:Left.Y1Time=200 ms //OK
MCU>PC:Left.Y2Time=300 ms //Ok
MCU>PC:Left.Y3Time=0 ms //Error, should be 40 ms
MCU>PC:Left.Reaction=0 ms //Error. should be 20 ms

What could be the problem in my code?
Ttelmah



Joined: 11 Mar 2010
Posts: 19520

View user's profile Send private message

PostPosted: Sat Nov 17, 2012 4:00 pm     Reply with quote

tmp, is already a pointer. You should load this with a value a few characters higher than the starting point you are working from in the string. Ideally the location of the space or the end of string. &tmp, is not the correct syntax, and it needs to be loaded with a value, above where you are in the string, or the search will return nothing. I'd suggest that &tmp, is giving a number that it below the locations of the last two commands.

If the commands are always in the same order, then it'd be quicker to search forward from where you have already got to, rather than keep searching from the start.

Best Wishes
asmboy



Joined: 20 Nov 2007
Posts: 2128
Location: albany ny

View user's profile Send private message AIM Address

PostPosted: Sat Nov 17, 2012 4:06 pm     Reply with quote

Quote:

clean_up_str()


standard c func ??

whats it supposed to do ??

Very Happy Very Happy Very Happy
Zloi



Joined: 26 Oct 2012
Posts: 11
Location: Croatia

View user's profile Send private message

PostPosted: Sun Nov 18, 2012 4:04 am     Reply with quote

asmboy wrote:
Quote:

clean_up_str()


standard c func ??

whats it supposed to do ??

Very Happy Very Happy Very Happy


It is function from example programs, EX_STR.C

This function cleans up the string. It removes all
punctuation and all multiple spaces. It also removes
any leading and trailing spaces.


Last edited by Zloi on Sun Nov 18, 2012 2:39 pm; edited 1 time in total
ckielstra



Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands

View user's profile Send private message

PostPosted: Sun Nov 18, 2012 6:21 am     Reply with quote

The same error as with the pointer tmp is present here:
Code:
*p=strstr(COMMAND,COMM);

P is an un-initialised pointer and you are modifying the contents of where this random pointer is pointing to. Because you are applying the same error everywhere the code is working more or less, until some other part of your program is accessing the same memory address.
These type of errors are common and difficult to debug. Many people have difficulties using pointers when they start programming in C. Pointers are very efficient but because of the programming errors many of the newer programming languages don't support them.

My old teacher at university teached me the following trick:
Whenever you see the '*' character you can replace it with the text 'the contents of' and for the '&' character 'the address of'.
So, when you see
Code:
*p=strstr(COMMAND,COMM);

This can be read as
Code:
'the contents of' p = strstr(COMMAND,COMM);
strstr returns a pointer and the correct syntax would have been :
Code:
p = strstr(COMMAND,COMM);


Fix this same error in the other 8 locations where you use p.
Ttelmah



Joined: 11 Mar 2010
Posts: 19520

View user's profile Send private message

PostPosted: Sun Nov 18, 2012 6:40 am     Reply with quote

You need to correct the way your pointers are working:
Code:

void MsgDecode (void){
   unsigned int16 TempNum;
   char *p;
   char *tmp;
   
   if(bit_test(FraimFlags,FrameDecode)){
      bit_clear(FraimFlags,FrameDecode);
      strcpy(COMMAND,COMMAND_Buf);
      clean_up_str(COMMAND);
      printf(bputc,COMMAND);
      tmp=COMMAND; //Must be preloaded or there is no guarantee what
      //it may contain. Could be null, which would stop it working....
     
      strcpy(COMM,"L1YE");                       
      p=strstr(COMMAND,COMM); //No * needed. strstr, returns a pointer
      if(p!=0){ //p is 0 if string not found _not_ *p
         TempNum=strtoul((p+strlen(COMM)),&tmp,10); //again no *
         Left.Y1Time=TempNum;
         printf(bputc,"Left.Y1Time=%Lu ms\r\n",Left.Y1Time);     
      }   
      //Now, 'tmp' points to where the last number ended. Use this
      strcpy(COMM,"L2YE");                       
      p=strstr(tmp,COMM); //Only search in the remaining string
      if(p!=0){ //again p, not *p
         TempNum=strtoul((p+strlen(COMM)),&tmp,10);
         Left.Y2Time=TempNum;
         printf(bputc,"Left.Y2Time=%Lu ms\r\n",Left.Y2Time);
      }   

      strcpy(COMM,"L3YE");                       
      p=strstr(tmp,COMM);
      if(p!=0){
         TempNum=strtoul((p+strlen(COMM)),&tmp,10);
         Left.Y3Time=TempNum;
         printf(bputc,"Left.Y3Time=%Lu ms\r\n",Left.Y3Time);
      }   

      strcpy(COMM,"LGRN");                     
      p=strstr(tmp,COMM);
      if(p!=0){
         TempNum=strtoul((p+strlen(COMM)),&tmp,10);
         Left.Reaction=TempNum;
         printf(bputc,"Left.Reaction=%Lu ms\r\n",Left.Reaction);
      }   
   bit_clear(FraimFlags,FraimCheck);
   bit_clear(FraimFlags,FrameCopy);
   }
}


Key things are you are using '*' in several places where you actually want the pointer, not the contents. Then you are continuously searching the whole string, rather than moving 'forwards' from where you have reached (this is the whole point of the tmp pointer 'filled' by the strtoul function).
strstr for instance, returns a null pointer, _not_ a pointer to null, when the value is not found.

Best Wishes
Zloi



Joined: 26 Oct 2012
Posts: 11
Location: Croatia

View user's profile Send private message

PostPosted: Sun Nov 18, 2012 12:56 pm     Reply with quote

Thank you all for your suggestion.

I corrected mistakes with pointers, function now works properly.

Best Regards
hmmpic



Joined: 09 Mar 2010
Posts: 314
Location: Denmark

View user's profile Send private message

PostPosted: Sun Nov 18, 2012 3:35 pm     Reply with quote

Hi
Maybe this:-)

Code:
signed int GetIntFToken(char *inps,char *token){
 int i;
 char *f1;
 char buff[5];

 buff[0]=0x00;
 f1=strstr(inps,token);//find token
 if (!f1){return (-1);}//exist ?
 f1=f1+strlen(token);//if found, add length of token
 for (i=0; f1[i]; i++){
  if (f1[i]!=' '&&i<4) {buff[i]=f1[i];} else {buff[i]=0x00; break;}
 }
i=atoi(buff);
 return (i);
}


void main(){
 char s1[]="L1YE200 L2YE303 L3YE40 LGRN20 ";

 R1=GetIntFToken(s1,"L1YEs");
 R2=GetIntFToken(s1,"L2YE");
 R3=GetIntFToken(s1,"L3YE");
 R4=GetIntFToken(s1,"LGRN");

 printf("%d %d %d %d",R1,R2,R3,R4);
}
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group