tags:

views:

155

answers:

3

guys,

had been years since I wrote my last line in C, now I'm trying to start again to make some function and use than as an extension for php.

this is an easy function to create "small urls"

lets say:

a0bg a0bf a0bh

the problem I'm having is when I have to "increment" an string like zzz then I got: Bus Error

otherwise if I increment abr for example I got the result: abs

at some point I think that my problem here is returning the string result

both code are fully functional as I posted them here.

to compile I'm using: *gcc append_id_test.c -o append_id*

I'm on OS X leopard

when I do this, it works: (please pay attention on the call to the function append_id)

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

char* append_id(char*);
int main(void) {
    char *x;
    char *incremented;

    x = "aaab";

    printf("%s\n", append_id(x)); // should print 00000 (lenght 5)


    incremented = (char *) malloc((strlen(x) + 2) * sizeof(char));
    incremented = append_id(x);

    printf("--->  %s\n", incremented); // should print 00000 (lenght 5)

}

char* append_id(char *id) {

    int x;
    char* new_id;
    int id_size = strlen(id);

    new_id = (char *) malloc((strlen(id) + 2) * sizeof(char));

    for ( x = 0; x < id_size; x++ )
    {
     new_id[x] = '0';
    }

    strcat(new_id, "0");

    return new_id;
}

but the whole code isn't working (AS you see the call to the append_id function was made in the same way as the above example)

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


char* append_id(char*);
char* increment_id(char*, int);
char* get_next_id(char*);


int main(int argc, char *argv[]) {
    char *x;
    int a;

    x = "zz";

    printf("incrementando %s -> %s\n", "zz", get_next_id(x));

    return 0;
}


char * get_next_id(char *last_id)
{ 
    int x, pos;
    char *next_id;
    char is_alnum = 1;

    // if the last id is -1 (non-existant), start at the begining with 0
    if ( strlen(last_id) == 0 )
    {
     next_id = "0";
    }
    else
    {

     // check the input
     for(x = 0; last_id[x]; x++) 
     {
      if(!isalnum(last_id[x]))
      {
       is_alnum = 0;
       break;
      }
     }

     if (is_alnum == 0)
     {
      return "";
     }


     // all chars to lowercase
     for(x = 0; last_id[x]; x++) 
     {
      last_id[x] = tolower(last_id[x]);
     }


     // loop through the id string until we find a character to increment
     for ( x = 1; x <= strlen(last_id); x++ )
     {
      pos = strlen(last_id) - x;

      if ( last_id[pos] != 'z' )
      {
       next_id = increment_id(last_id, pos);
       break; // <- kill the for loop once we've found our char
      }
     }

     // if every character was already at its max value (z),
     // append another character to the string
     if ( strlen(next_id) == 0)
     {
      next_id = (char *) malloc((strlen(last_id) + 2) * sizeof(char));
      next_id = append_id(last_id);
     }

    }


    return next_id;
}



char* append_id(char *id) {

    int x;
    char* new_id;
    int id_size = strlen(id);

    new_id = (char *) malloc((strlen(id) + 2) * sizeof(char));

    for ( x = 0; x < id_size; x++ )
    {
     new_id[x] = '0';
    }

    strcat(new_id, "0");

    return new_id;
}



char* increment_id(char *id, int pos){
    char current, new_char;
    char * new_id ;
    int x;

    new_id = (char *) malloc((strlen(id) + 1) * sizeof(char));


    current = id[pos];

    if ( current >= 0x30 && current <= 0x39 )
    {
     if ( current < 0x39 )
     {
      new_char = current + 1;
     }
     else // if we're at 9, it's time to move to the alphabet
     {
      new_char = 'a';
     }
    }
    else // move it up the alphabet
    {
     new_char = current + 1;
    }


    for ( x = 0; x < strlen(id); x++ )
    {
     if (x == pos) {
      new_id[x] = new_char;
     }
     else {
      new_id[x] = id[x];
     }
    }


    // set all characters after the one we're modifying to 0
    if ( pos != (strlen(new_id) - 1) )
    {
     for ( x = (pos + 1); x < strlen(new_id); x++ )
     {
      new_id[x] = '0';
     }
    }

    return new_id;
}
A: 

I will probably check the for loops in get_next_id. You are iterating the pointer without a limit. Where does this ends?

    // check the input
    for(x = 0; last_id[x]; x++) 
    {
            if(!isalnum(last_id[x]))
            {
                    is_alnum = 0;
                    break;
            }
    }

Do the same you did on append_id. Get the size of the received pointer (int id_size = strlen(id);) then iterate the last_id pointer based on that length.

I think what happens is that your loop never ends. last_id[x] will return a character, not a condition. So it will be always true, unless there is the case that the current character is zero.

Freddy
C strings are null terminated. last_id[x] will be 0 at the end of a string.
p00ya
I think that is a problem, since char* x="zz" is not null terminated (at least could not be assured). He is assigning a value of "zz" (whatever that means) into pointer x. I don't even know what address that will be.
Freddy
+1  A: 

Other than forgetting to free your memory, you have two issues, on lines 12 and 72. On line 12, you must declare "zz" as a character array, not a string literal:

    char x[] = "zz";

On line 72, next_id will be null if all characters are at their max value. strlen(null) will cause another segfault.

    if ( !next_id )
ACoolie
+3  A: 

Please compile with -Wall next time ;)

To start with, before using tolower and isalnum,

#include <ctype.h>

Second, you assign x (in main scope) to point at a string literal, which is const. Hence you will get a memory violation when you try to write over it when lowercasing. Try initialising to, say,

char x[] = "zz";

You should also initialise next_id in get_next_id:

char *next_id = NULL;

Later, don't run strlen on a null pointer:

// if every character was already at its max value (z),
// append another character to the string
if (!next_id || !strlen(next_id))
{
    next_id = append_id(last_id);
}

In append_id, you need to null terminate before using strcat:

for ( x = 0; x < id_size; x++ )
{
    new_id[x] = '0';
}
new_id[id_size] = 0;

That just gets it to work. There are a lot of things wrong with your code. You mention

had been years since I wrote my last line in C

I really hope you had a better grasp of it years ago, seems like you're oblivious to a lot of fundamental C memory, string and pointer concepts.

p00ya
During development, I often run with `-pedantic -Wall -Wextra -Wshadow`, which is a bit noisy, but still useful.
ephemient