Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am mostly interested in how the parse_string function could be improved. But any comments on my test strategy also welcome.

The input is a string which can be either all digits in which case the result should be the digits string OR there can be string formatting outside angle brackets which must be removed. the digit string is enclosed within the angle brackets in that case.

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

char* parse_string(const char* s, char* parsed) {
   /* sample string for angle brackets */
   const char* p = s;
   int lbrack = -1, rbrack = -1;
   while(*p) {
      if(*p == '<' && lbrack == -1)  /* grab first < */
         lbrack = p - s;
      else if(*p == '>') /* and last > char */
         rbrack = p - s;

      ++p;
   }

   /* strip everything BEFORE < and AFTER > if have brackets and in correct order */
   if(lbrack != -1 && rbrack != -1 && lbrack < rbrack) {
      memcpy(parsed, s+lbrack+1, rbrack-1-lbrack);
      parsed[rbrack-1-lbrack] = '\0';
   }
   else {
      strcpy(parsed, s ? s : "");
   }

   return parsed;
}

int main() {

   char buf[100];
   /* 4 normal cases, error cases  */
   const char* input[] = { "N<737867>Sales Line 1", "123456", "", "<123>", "<", ">", "", "<<<<123>", "<123>>>>", "<123", "123>" };
   const char* output[] = { "737867", "123456", "", "123", "<", ">", "", "<<<123", "123>>>", "<123", "123>" };
   size_t sz = sizeof(input) / sizeof(input[0]);

   size_t i;
   for(i = 0; i < sz; ++i) {
      printf("Test string \"%s\" => \"%s\", %s\n", input[i], output[i], strcmp(output[i], parse_string(input[i], buf)) == 0 ? "passed" : "failed");
   }
   return 0;
}
share|improve this question
    
Why the s test in strcpy(parsed, s ? s : "")? Had s been NULL, while(*p) would have died earlier. –  chux Dec 20 '14 at 22:05

1 Answer 1

up vote 2 down vote accepted
  1. Add a buffer length parameter and then use strncpy to avoid overflow.

  2. Your testing code hides the usage of the function. It's clearer to put it on a separate line:

    for(i = 0; i < sz; ++i) {
        parse_string(input[i], buf)
        printf("Test string \"%s\" => \"%s\", %s\n", 
                     input[i], output[i], strcmp(output[i], buf) == 0 ? "passed" : "failed");
    }
    
  3. You missed the out of order cases like >123<
share|improve this answer
    
strncpy() is easy to get wrong. Suggest posting suggested code, else OP is likely to just trade a strcpy() problem for a strncpy() one. –  chux Dec 20 '14 at 22:07

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.