0

Here is my function's code:

char * increment_b_string(char * b_string)
{
    char * ret_string = (char *) malloc(7);

    ret_string = "001aaa";

    if (*(b_string + 2) == '9')
    {
        *(ret_string +2) == '0';

        if (*(b_string + 1) == '9')
        {
            *(ret_string +1) = '0';
            *(ret_string) = *(b_string) + 1;
        } else {
            *(ret_string + 1) = *(b_string + 1) + 1;
        }
    } else {
        *(ret_string + 2) = *(b_string + 2) + 1;
    }

    return ret_string;
}

Any thoughts as to why it might be failing?

The general idea is that b_string will contain a value like "123aaa". The "aaa" portion does not matter, and will never change. Only the first 3 numbers will. They need to increment as though they were integers. Leading zeroes need to be preserved in the beginning. So, if input is "004aaa", output should be "005aaa". This only needs to go up to "300aaa", hence why I'm not considering anything more than that. It's a school programming assignment, hence the very contrived nature of the problem.

Thanks.

Edit: I've changed my function. Here is what it is now.

void increment_b_string(char * b_string)
{   
    if (b_string[2] == '9')
    {
        b_string[2] == '0';

        if (b_string[1] == '9')
        {
            b_string[1] = '0';
            b_string[0] += 1;
        } else {
            b_string[1] += 1;
        }
    } else {
        b_string[2] += 1;
    }
}

Assuming that b_string is initially populated with...

strcpy(b_string, "001aaa");

...would this be correct? My program is still exhibiting the same behavior, but this may be an error somewhere else in the code.

rybosome
  • 5,046
  • 7
  • 44
  • 64
  • I should add that by "failing", I mean that the function will essentially freeze. Putting in printf("Breakpoint") in various places will only output to the terminal before the first if statement. – rybosome Aug 17 '10 at 18:07
  • 1
    Note that `*(ret_string + 2)` means exactly the same thing as `ret_string[2]`. It's defined that way. – David Thornley Aug 17 '10 at 18:13

6 Answers6

14

You cannot increment or change a string literal - they are read only in C, which is what you're doing.

char * ret_string = (char *) malloc(7);
ret_string = "001aaa";

Does not do what you think it does, it does not copy the string into the malloced spaceret_string points to, it sets ret_string to point to the string literal "001aaa". Use this instead:

 char * ret_string = (char *) malloc(7);
 strcpy(ret_string,"001aaa");
nos
  • 223,662
  • 58
  • 417
  • 506
  • 1
    Also, make sure you include string.h when using strcpy – rownage Aug 17 '10 at 18:10
  • You also probably want `*(ret_string + 2) == '0';` to have only one equals sign. And your algorithm will do something funny for `'999aaa'` – David Brown Aug 17 '10 at 18:17
  • That should fix the problem, then you just need to take care of the memory leak (unless it's handled elsewhere). – ssube Aug 17 '10 at 18:19
  • 2
    Rather than using a separate malloc and strcpy you may as well use strdup. For example `char *ret_string = strdup("001aaa");` – torak Aug 17 '10 at 18:26
2

For one, I would initialize ret_string through:

char * ret_string = strdup("001aaa");

You currently assign ret_string to a string literal, which you don't want to do when you are going to modify the string.

Note that you should call free(ret_string) when you are done with it. The implementation of strdup contains a malloc.

Second, you may have an easier time going through the logic of the function if you use the syntax b_string[i] instead of *(b_string + i). They are equivalent.

Edit:

Since strdup isn't ANSI C, you could always define your own function with the same functionality, something like:

char *strdup (const char *s) {
    char *d = (char *)(malloc (strlen (s) + 1));
    if (d == NULL) return NULL;
    strcpy (d,s);
    return d;
}

Reference:

strdup() - what does it do in C?

Community
  • 1
  • 1
Justin Ardini
  • 9,768
  • 2
  • 39
  • 46
  • I'm a big fan of strdup also, but it should be noted: a) it uses malloc, so someplace we need to call `free(ret_string)`, b) It's part of the POSIX standard, not the ANSI Standard, and therefore may not be in your compiler's library. c)Microsoft C, at least, has switched to calling it `_strdup` – James Curran Aug 17 '10 at 18:14
  • I noted (a) in my answer, and (b) and (c) are very good points. :) If you **can** use `strdup`, this is a good place to do so. – Justin Ardini Aug 17 '10 at 18:15
1

None of this has been tested, but the following should work well.

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

/* Many libc implementations have strdup, but some don't */
char * strdup(const char * str) {
     char * copy = malloc(strlen(str)+1);
     if (copy) {
         strcpy(copy, str);
     }
     return copy;
}


/* This increments the first ASCII integer it finds within the string by one.
 * The integer is treated as though it is base 10 for the purposes of incrementing
 * and in the event that the upper digit is '9' before the increment it is '0' after
 * the increment and the carry digit is lost.
 */ 
char * increment_b_string_inplace(char * str) {
     char * s = str;
     while (*s) {  /* Look for where the number starts. */
        char c = *s;
        if (isdigit(c)) { 
            /* This is the start of a number! */
            char * e = s+1;
            unsigned carry;
            while (isdigit(*++e) ) {;} /* find where the number ends */ 
            e--;  /* we went 1 too far, so back up */
            do {   /* Do the actual increment. ][
                unsigned i = *e - '0';
                i++;
                carry = i % 10; /* this should always be 0 or 1 */
                *e = (char)('0' + (i / 10) );
                e--;
            } while (e<=s && carry);
        }
    }
    return str;
}


/* This is the function you originally wanted. */
char * increment_b_string(const char * b_string)
{
    char * str = strdup(b_string);

    if (!str) {
        return str;
    }
    return increment_b_string_inplace(str);
}

You could also have read the entire integer into an int variable, incremented that, and then turned that back into a string.

nategoose
  • 12,054
  • 27
  • 42
1

Below is code to achieve what I think you're wanting. Included is a re-write of your increment_b_string. It uses strtol (string to long) and sprintf to convert from string to int and back, respectively. An assumption is that the character string on which increment_b_string will operate is always of dddccc format (d = digit, c = character).

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

char * increment_b_string(char * b_string)
{
    char * ret_string;
    int   int2inc;

    ret_string = malloc(sizeof(b_string));
    int2inc = (int) strtol(b_string, ret_string, 10);
    int2inc ++;
    sprintf(ret_string, "%03d%s", int2inc, &b_string[3]);
    return ret_string;
}

int main(void) {
    char the_string[7] = "000aaa";
    printf("before, the_string = \"%s\"\n", the_string);
    strcpy(the_string, increment_b_string(the_string));
    printf("after, the_string = \"%s\"\n", the_string);
}

The output is:

before, the_string = "000aaa"
after, the_string = "001aaa"

GreenMatt
  • 18,244
  • 7
  • 53
  • 79
0

Also, your ret_string will have one or two zeroes at the front that should be something else, if the else clauses are exercised. To fix this, you need to initialize ret_string from b_string, not from a constant:

char * ret_string = strdup(b_string);
LarsH
  • 27,481
  • 8
  • 94
  • 152
0

a solution with strictly C89, without malloc/strdup and more flexible for variable input-strings:

char *inc_bstr(char *str)
{
  char *e,f[] = "%03ld%s";
  long i = strtol(str,&e,10)+1;
  f[2] = '0'+(e-str);
  sprintf(str,f,i>300?300:i,e);
  return str;
}

...
char x1[]="0aaa";
puts(inc_bstr(x1));
...
char x2[]="00aaa";
puts(inc_bstr(x2));
...
char x3[]="000aaa";
puts(inc_bstr(x3));
...
char x4[]="0000aaa";
puts(inc_bstr(x4));
...
char x5[]="00000aaa";
puts(inc_bstr(x5));
...
char x6[]="0000bb";
puts(inc_bstr(x6));
...
char x7[]="000";
puts(inc_bstr(x7));
...
char x8[]="000xxx",z=99;
while( z-- )
  puts(ins_bstr(x8));
user411313
  • 3,930
  • 19
  • 16