Undefined Behavior

Discussion in 'C Programming' started by bbaale, Apr 25, 2007.

  1. bbaale

    bbaale Guest

    Why would this code cause undefined behavior?



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

    #define TRUE 1
    #define FALSE 0
    #define MAX_PASSWORD 11
    int loop;
    unsigned char rx;


    int _strlen(s)
    unsigned char s[];
    {
    int i;

    i=0;
    while(s!='\0')
    ++i;
    return i;
    }

    int encrypt (s, x, len)
    unsigned char *s ;
    unsigned char x;
    int len;
    {

    int i,ok;
    unsigned char seed_1,seed_2;

    seed_1 = rx;
    seed_2 = rx + 31;
    ok = TRUE;
    for(i = 0 ; i < len ; i++)
    {
    seed_1 -= i;
    seed_2 += i;
    if(i%2 != 0)
    s -= seed_1;
    else
    s += seed_2;
    if(s == '&' || s== '\0' || s == '\n')
    ok = FALSE;
    }
    if (x == '&')
    ok = FALSE;
    return ok;
    }



    int decrypt (s, len)
    unsigned char *s;
    int len ;

    {
    unsigned char seed_1, seed_2;
    int i;

    seed_1 = rx;
    seed_2 = rx+31;
    for (i = 0; i < len ; i++)
    {
    seed_1 -= i;
    seed_2 +=i;
    if (i % 2 != 0)
    s += seed_1;
    else
    s -= seed_2;
    }
    return TRUE;
    }



    int encrypt_string (s)
    unsigned char *s ;
    {

    unsigned char x;
    int c, len;

    x=0;
    c=FALSE;
    len= _strlen(s);

    while( c!= FALSE || loop>0)
    {
    x++;
    c=encrypt(s, x, len);
    loop--;
    }
    s[len++] = x;
    s[len++] = rx;
    s[len]= '\0';

    return TRUE;
    }


    int decrypt_string(s)
    unsigned char *s;
    {

    unsigned char x,y ;
    int len;

    len = _strlen(s)-1;

    rx= s[len];
    len--;
    x=s[len];
    for(y = 0; y < x ; y++)
    decrypt(s, len);
    s[len]= '\0';

    return TRUE;
    }


    int main() {

    unsigned char *s;
    unsigned char endchar;
    unsigned int y;
    int len;
    s=(unsigned char *) calloc( MAX_PASSWORD, sizeof(unsigned char) );
    if(s==NULL){
    printf("Could not allocate Memory");
    return 0;
    }

    fgets(s,MAX_PASSWORD,stdin);
    len=_strlen(s);

    endchar =s[len-1];

    if(endchar=='\r' || endchar=='\n')
    s[len-1]='\0';

    rx=strlen(s) ;
    loop=1;

    y=encrypt_string(s);

    if(y==TRUE)
    printf("\n\nEncrypted String :%s ######%d\n\n",s,_strlen(s));

    y=decrypt_string(s);
    if(y==TRUE)
    printf("Decrypted String: %s %d\n",s,_strlen(s));

    if(*s=!NULL){
    free(s);
    s=NULL;
    }
    else
    printf("Could not deallocate memory\n");
    return 0;

    }
     
    bbaale, Apr 25, 2007
    #1
    1. Advertisements

  2. said:
    Invading implementation namespace.
     
    Richard Heathfield, Apr 25, 2007
    #2
    1. Advertisements

  3. What's this _strlen()? Apart from anything else, that's a reserved
    name.
    So at this point s points to a string up to 11 bytes long (including
    the nul at the end) in a buffer 11 bytes long.
    Woops! You're adding things on to the end of s, and overflowing the
    buffer.

    You should be able to find mistakes like this yourself, using a
    memory checking tool such as valgrind.

    -- Richard
     
    Richard Tobin, Apr 25, 2007
    #3
  4. bbaale

    CBFalconer Guest

    Because right here you have used a label reserved for the
    implementation. Don't do that.
     
    CBFalconer, Apr 25, 2007
    #4
  5. It was hard to resist rewriting this mess. Having resisted, I still
    refuse to quote it all, except to note that
    probably is not what you meant.
    if (*s = !NULL) ?* use some whitespace, for pete's sake */
    does not mean the same thing as what you probably meant,
    if (*s)
     
    Martin Ambuhl, Apr 25, 2007
    #5
  6. bbaale

    Gregor H. Guest

    Should read


    if (s != NULL)
    {
    free(s);
    s = NULL;
    }
    else
    printf("Could not deallocate memory\n");


    I guess.


    G.
     
    Gregor H., Apr 25, 2007
    #6
  7. I wouldn't think so; I see no reason why a dynamically allocated
    string shouldn't be freed simply because its length is 0.
     
    Christopher Benson-Manica, Apr 25, 2007
    #7
  8. You're right. But guessing what the OP meant by his assignment
    statement is up in the air. Any guess is likely to be wrong.
     
    Martin Ambuhl, Apr 26, 2007
    #8
  9. bbaale

    Gregor H. Guest

    Not so!

    It should read


    if (s != NULL)
    {
    free(s);
    s = NULL;
    }
    else
    printf("Could not deallocate memory\n");


    I guess. (Such a construction -without the else-path- does make sense
    _if_ it is possible that the allocated memory might have been freed
    already at an earlier time. I once used such a construction, IIRC.)


    G.
     
    Gregor H., Apr 26, 2007
    #9
  10. You can feed free() with anything that got returned by malloc(), calloc() or
    realloc() which in turn can return NULL, so you can safely call free(NULL)
    unconditionally and safe that exta code.

    free(s),s = NULL;

    The mesages "Could not deallocate memory" is a) not neccessary (as you
    haven't got any memory to free in the first place) and b) if at all should
    go to stderr, shouldn't it?

    Bye, Jojo
     
    Joachim Schmitz, Apr 26, 2007
    #10
  11. bbaale

    CBFalconer Guest

    But, since free can operate on NULL, it can all be replaced by:

    free(s);
     
    CBFalconer, Apr 26, 2007
    #11
  12. bbaale

    Gregor H. Guest

    Ah, good to know! :)


    G.
     
    Gregor H., Apr 26, 2007
    #12
  13. I'm not sure what the benefit is. Aside from the fact that in OP's
    code s had already been checked against NULL, if s is NULL there is no
    memory to deallocate, making the error message rather strange. In
    any case it seems to me that what is wanted is something like

    void debug_free( void **ptr ) {
    if( *ptr != NULL ) {
    free( *ptr );
    *ptr = NULL;
    }
    else {
    fprintf( stderr, "Warning: ptr may have been free'd more than once");
    }
    }

    Otherwise I see little point in not simply replacing the code with

    free( s );
    s = NULL;
     
    Christopher Benson-Manica, Apr 26, 2007
    #13
  14. bbaale

    pete Guest

    How is s going to equal NULL at that point in code,
    when you've got This code first?:

    s=(unsigned char *) calloc( MAX_PASSWORD, sizeof(unsigned char) );
    if(s==NULL){
    printf("Could not allocate Memory");
    return 0;
    }
     
    pete, Apr 28, 2007
    #14
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.