0

Below is a section of code that I use for parsing tokens.

There is a line indicated with >>>> near the bottom that is no longer required, but if I comment it out, the cmd_parse_value_lookup() function fails. If i leave it in, the code runs properly. Could anyone tell me why, and explain what is happening?

void cmd_parse(void)
  {
    cmd_parse_value=0;  
    int cmd_parse_counter = 1;
    char *cmd_parse_pointer;
    cmd_parse_pointer = strtok(cmd_buffer_in, " ");
    if (cmd_parse_pointer!=NULL)
    {
      cmd_parse_value_lookup(cmd_parse_pointer);
    }
    while (cmd_parse_pointer != NULL)
    {
      cmd_parse_counter++;
      cmd_parse_pointer = strtok(NULL, " ");
      if (cmd_parse_pointer!=NULL)
      {
   >>>>cmd_buffer_sprintf_return = sprintf(cmd_buffer_sprintf,"%i: %s\r\n", cmd_parse_counter, cmd_parse_pointer);  //WHY DO I NEED THIS LINE
        cmd_parse_value_lookup(cmd_parse_pointer);
      }
    }
  }

void cmd_parse_value_lookup(char *cmd_command)
{
  if (strcmp(cmd_command,"show")==0)
  {
    cmd_parse_value |= 1;
  }
  else if (strcmp(cmd_command,"get")==0)
  {
    cmd_parse_value |= 1;
  } 
  else if (strcmp(cmd_command,"set")==0)
  {
    cmd_parse_value |= 2;
  }
  else if (strcmp(cmd_command,"system")==0)
  {
    cmd_parse_value |= 4;
  }
}

Edit: This is the full code:

/** C O M M A N D ************************************************************/

#include "generic.h"
#include <stdio.h>
#include <string.h>

void btm_out_character (char character);
void btm_out_string(char *string);
void cmd_parse(void);

char cmd_buffer_in[81]="\0                                                                         ";
int cmd_buffer_in_position=0;
unsigned long long cmd_parse_value=0;

char cmd_buffer_sprintf[81];
int cmd_buffer_sprintf_return;

void cmd_parse_value_lookup(char *cmd_command);
void cmd_buffer_in_add(int character);

void cmd_init(void)
{

}

void cmd_cls(void)
{
    if (dbg_mode==1){btm_out_string("\033[2J\033[1;33;40m\033[H\r\n");}
    if (dbg_mode==1){btm_out_string("================================================================================");}
    if (dbg_mode==1){btm_out_string("\033[24;0H================================================================================");}
    if (dbg_mode==1){btm_out_string("\033[1;36;40m\033[8;66H DEBUG MODE ");}
    if (dbg_mode==1){btm_out_string("\033[24;7H ***** ******** ********** Engineering Limited, All Rights Reserved ");}
    if (dbg_mode==1){btm_out_string("\033[10;0H");}
}   

void cmd_prompt(void)
{
    cmd_buffer_in_position = 0; // Clear buffer position
    cmd_buffer_in[0]=0;  // Clear buffer
    if (dbg_mode==1){btm_out_string("\r\n\033[1;37;40m***:> ");}    
}

void cmd_buffer_in_add(int character)
{
    switch (character)
    {
        case 8:
            if (cmd_buffer_in_position>0)
            {
                if (dbg_mode==1){btm_out_string("\b \b");}
                cmd_buffer_in[(int)cmd_buffer_in_position-1] = (char)character;
                cmd_buffer_in[(int)cmd_buffer_in_position] = 0;
                cmd_buffer_in_position--;
            }
                else
            {
                if (dbg_mode==1){btm_out_character(7);}     //BELL alert (too long a string)
                cmd_buffer_in[0] = 0;
                cmd_buffer_in_position = 0;
            }
            break;
        case 13:
            cmd_parse();
            cmd_buffer_sprintf_return = sprintf(cmd_buffer_sprintf,"\r\nparse value=%llu\r\n", cmd_parse_value);
            if (dbg_mode==1){btm_out_string(cmd_buffer_sprintf);}
            cmd_prompt();
            break;
        default:
            if (cmd_buffer_in_position<73)
            {
                if (dbg_mode==1){btm_out_character((char)character);}       //Echo character to host
                cmd_buffer_in[(int)cmd_buffer_in_position] = (char)character;
                cmd_buffer_in[(int)cmd_buffer_in_position+1] = 0;
                cmd_buffer_in_position++;
            }
            else
            {
                if (dbg_mode==1){btm_out_character(7);}     //BELL alert (too long a string)
            }
    }//switch
}//cmd_buffer_in_add

void cmd_parse(void)
{
    cmd_parse_value=0;

    /* this was code for testing */
    //cmd_buffer_sprintf_return = sprintf(cmd_buffer_sprintf,"\r\nClone %s",cmd_buffer_in);
    //cmd_buffer_sprintf_return = sprintf(cmd_buffer_sprintf,"Returnval='%i''%i''%i'",cmd_buffer_in[(int)0],cmd_buffer_in[(int)1],cmd_buffer_in[(int)2]);
    //if (dbg_mode==1){btm_out_string(cmd_buffer_sprintf);}

    int cmd_parse_counter = 1;
  char *cmd_parse_pointer;

  cmd_parse_pointer = strtok(cmd_buffer_in, " ");
  if (cmd_parse_pointer!=NULL)
    {
    cmd_parse_value_lookup(cmd_parse_pointer);
  }
  while (cmd_parse_pointer != NULL)
  {
      cmd_parse_counter++;
    cmd_parse_pointer = strtok(NULL, " ");
    if (cmd_parse_pointer!=NULL)
    {
        cmd_buffer_sprintf_return = sprintf(cmd_buffer_sprintf,"%i: %s\r\n", cmd_parse_counter, cmd_parse_pointer);  //WHY DO I NEED THIS LINE
            //if (dbg_mode==1){btm_out_string(cmd_buffer_sprintf);}
            cmd_parse_value_lookup(cmd_parse_pointer);
    }
  }
}

void cmd_parse_value_lookup(char *cmd_command)
{
    if (strcmp(cmd_command,"show")==0)
    {
        cmd_parse_value |= 1;
    }
    else if (strcmp(cmd_command,"get")==0)
    {
        cmd_parse_value |= 1;
    } 
    else if (strcmp(cmd_command,"set")==0)
    {
        cmd_parse_value |= 2;
    }
    else if (strcmp(cmd_command,"system")==0)
    {
        cmd_parse_value |= 4;
    }
    else if (strcmp(cmd_command,"sys")==0)
    {
        cmd_parse_value |= 4;
    }
    else if (strcmp(cmd_command,"adc")==0)
    {
        cmd_parse_value |= 8;
    }
    else if (strcmp(cmd_command,"a2d")==0)
    {
        cmd_parse_value |= 8;
    }
    else if (strcmp(cmd_command,"channel1")==0)//
    {
        cmd_parse_value |= 16;
    }
    else if (strcmp(cmd_command,"ch1")==0)
    {
        cmd_parse_value |= 16;
    }
}
Graham
  • 318
  • 1
  • 16
  • Tough to tell, as we can't see the declaration of `cmd_buffer_sprintf` or see what it gets set to, but if it points into `cmd_buffer_in` somewhere, it's not surprising that deleting the line changes the results. – Chris Dodd Aug 13 '12 at 22:04
  • I have added the rest of the code if that helps – Graham Aug 13 '12 at 22:17
  • 1
    Please explain what do you mean by "fails". That term is totally useless when asking a question. We want to know: seg fault, bad return value, ... ? It's like going to a mechanic and saying "my car is making a weird noise, what's wrong with it?" The mechanic wants to know what kind of weird noise. – Marlon Aug 13 '12 at 22:22
  • Sorry, i tried to think of everything, but always forget something. cmd_parse_value always equals 0 if I comment out that line. but with the line in, it generates a code equivalent to the words enter (as it should) – Graham Aug 13 '12 at 22:28

4 Answers4

0

Since you don't use cmd_buffer_sprintf anywhere else, that line of code seems to be useless. It might be the case that cmd_buffer_sprintf is a global var that you use somewhere else, but in the piece of code you presented, that line doesn't change anything.

Razvan
  • 9,925
  • 6
  • 38
  • 51
  • I still cannot identify a problem. Why don't you try to debug it ? – Razvan Aug 13 '12 at 22:27
  • how would I do that? its c30 that runs on a PIC24EP512GU810. I did add that I am inexperienced in C to my original post, although someone edited it out, I guess it was deemed unnecessary. I have spent a few hours on it before posting the question on here. I found working with strings in C very difficult. Thanks for looking at my code though. – Graham Aug 13 '12 at 22:31
0

It's possible that cmd_buffer_sprintf isn't initialized if you take that line out.

Try adding a *cmd_buffer_sprintf = '\0' somewhere in main() and see if that corrects the problem.

tomlogic
  • 11,489
  • 3
  • 33
  • 59
  • if it's not initialized in the cmd_parse(), it will be in the next line of CASE 13. Since cmd_buffer_sprintf is not used in the cmd_parse_value_lookup(), that shouldn't be the reason for the failure. – Razvan Aug 13 '12 at 22:26
0

Well thanks for the downvote, I am sure that whoever gave me it would have immediately guessed that there is a compiler bug that caused this issue, even if they were just learning C.

Anyway, the answer to the problem was that the compiler has sketchy (at best) support for unsigned long longs. For some reason that line let it work. I have re-written the section avoiding the use of unsigned long longs, and everything is now sorted. Thanks for your support trying to solve this.

Graham
  • 318
  • 1
  • 16
  • There's about a one in a million chance this is a compiler bug; what's much more likely is that you've invoked undefined behavior somewhere, and this extra call is perturbing the generated code sufficiently to mask it. – R.. GitHub STOP HELPING ICE Aug 14 '12 at 03:17
  • The problem is with the library that contains sprintf, there is a discussion regarding it on microchips forums. I used compiler, that was my bad - i still dont get the whole libraries thing. But yeah, the problem had absolutely nothing to do with my code :) – Graham Aug 14 '12 at 06:33
  • 1
    @Graham: can you post a link to the discussion you refer to? It'll help future visitors – Martin Thompson Aug 14 '12 at 08:48
0

We can see that the var cmd_parse_counter used on the line you try to comment becomes useless once you've commented the line out.

The logic behavior for the compiler is then to remove it from the assembly code.

This probably fix a buffer overflow or stack error that is happening somewhere else in the code.

To test this theory, declare cmd_parse_counter as volatile int cmd_parse_counter and comment out the line and check if the code runs or not. If it does run, then you can go further to find the issue. It might be related to your long long as you have spotted.

Another thing to check is that you are filling your buffer with spaces and then you are splitting using space token. Each split you do a strcmp. If your buffer doesn't end by 0 then you are certainly going to crash by using strcmp without a 0 char. You can try to init your buffer with all 0 using a for loop.

Damien
  • 1,492
  • 10
  • 32