Thread: Problem with pointers/strings/memory

  1. #1
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8

    Problem with pointers/strings/memory

    I've been pouring over this for over a day now and still have no solution, so hopefully I'll get one here

    I have a struct
    Code:
    struct MemoryStruct {
      char *memory;
      size_t size;
    };
    which contains a large string with several newlines and the size of that string in bytes.

    The program can read the string fine, and it works with printf no problem. However when I try to alter it (eg. using strcat) or use strtok to parse it (which is what i need to do), I get a segmentation fault at runtime. This also happens if I copy the data to a new string and try to perform the same operations on it.

    I've pasted the relavent bits of code below:
    Code:
        char tmp[4],url[255]="",fileline[255]="",*thedata,tmpstr1[2048];
        struct MemoryStruct chunk;
        chunk.memory=NULL;
        chunk.size = 0;    // no data at this point
    //...
    // read data into chunk...
    // ...
        chunk.memory=&chunk.memory[1]; // strip initial newline char.
        strncpy(tmpstr1, chunk.memory, strlen(chunk.memory) - 713); // strip adverts
        strcpy(chunk.memory,tmpstr1);
    // then either I get a segfault from any kind of write access.
    // I have tried appending '\0' to the end of the string after chopping the last 713
    // chars off, but that doesn't make any difference.
    Thanks in advance

    - Mitch

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    You're not allocating memory.

    chunk.memory = malloc( 1024 );
    For example.
    Then read up to 1024 bytes into the memory.

    Is your data always '\0' terminated, like a proper C-string should be?
    It needs to be if things like strcpy, strlen, strtok are to work.

    > chunk.memory=&chunk.memory[1];
    This is very bad - never modify a pointer to memory you allocated. You'll find it extremely hard to free that memory in future.
    The most correct thing to do here is use memmove() to shuffle the entire string back one place in memory.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    Quote Originally Posted by Salem
    You're not allocating memory.
    chunk.memory = malloc( 1024 );
    For example.
    Then read up to 1024 bytes into the memory.
    The data is created with these functions:
    Code:
    void *myrealloc(void *ptr, size_t size)
    {
      // There might be a realloc() out there that doesn't like reallocing NULL pointers, so we take care of it here 
      if(ptr)
        return realloc(ptr, size);
      else
        return malloc(size);
    }
    
    size_t WriteMemoryCallback(void *ptr, size_t size, size_t nmemb, void *data)
    {
      register int realsize = size * nmemb;
      struct MemoryStruct *mem = (struct MemoryStruct *)data;
    
      mem->memory = (char *)myrealloc(mem->memory, mem->size + realsize + 1);
      if (mem->memory) {
        memcpy(&(mem->memory[mem->size]), ptr, realsize);
        mem->size += realsize;
        mem->memory[mem->size] = 0;
      }
      return realsize;
    }
    It's for use with the cURL C Libraries if you know them at all.

    Quote Originally Posted by Salem
    Is your data always '\0' terminated, like a proper C-string should be?
    It needs to be if things like strcpy, strlen, strtok are to work.
    Yes, and as I said, I did try appending a new \0 to the string and then using the functions.
    Quote Originally Posted by Salem
    > chunk.memory=&chunk.memory[1];
    This is very bad - never modify a pointer to memory you allocated. You'll find it extremely hard to free that memory in future.
    The most correct thing to do here is use memmove() to shuffle the entire string back one place in memory.
    Thanks for the tip, this is my first project in C (personal as opposed to homework )

  4. #4
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    Code:
    chunk.memory=&chunk.memory[1]; // strip initial newline char.
    Probably not your problem, but this is kind of dangerous. You lose the original buffer, so if you attempt to reall() or free() the buffer you will get an error. Granter you moved 1 byte so undoing this isn't so bad.
    Code:
    strncpy(tmpstr1, chunk.memory, strlen(chunk.memory) - 713); // strip adverts
    strncopy() wants the length of the buffer tmpstr1, the way you are setting it is based on chunk.memory so if the value you set there is too large, it will try to overrun the tmpstr1 buffer.

    How are you setting the sizes of chunk.memory and tmpstr1?

    Edit: YOu guys are quick, got two posts in while I was typing this.

  5. #5
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    chunk.memory is allocated as in my previous post and tmpstr1 is declared as
    Code:
    char tmpstr1[2048];
    the sample data I'm using is only a few hundred chars.

  6. #6
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    A few hundred? Can we get specifics on that?

    if strlen(chunk.memory) is say 300 you get 300 - 700 pased to strncpy(), which will wrap around to a very large integer that definately can be overrunning tmpstr1

  7. #7
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    it's 448 chars. if it was overflowing, why would printf still print all the data upto the end?
    + that's not the point where i get the segfault. the error occurs after all that code and i try to operate on the string.

  8. #8
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    The way strncpy() works (according to the docs) is that it copies the buffer from chunk.memory to tmpstr1 until that length is reached. If that length is not reached it fills i nthe rest of the values in tmpstr1 with '\0' characters. So when you read from chunk.memory everything was fine, since it stopped at the '\0' to end the string. strncpy() stopped there as well, but it didn't stopp writing to tmpstr1 untill 448 - 713 (not sure what the unsigned integer version of that is) was reached

  9. #9
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    I've just tried assigning and writing tempstr1 with malloc() and the appropriate size and i still get the error.

  10. #10
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    I don't think I'm explaining this well. Whats going on is that strncpy() is trying to copy 4,294,967,031 (give or take) characters from chunk.memory into tmpstr1. With chunk.memory thats ok because strncpy() stops looking at it once a '\0' is reached. It does not stop there on tmpstr1 though. It writes that many bytes into tempstr1, when it reaches the end of chunk.memory it just writes '\0' into tmpstr1 until it writes all of the bytes. I don't think you can get a buffer large enough to hold that.

    FYI: I arrived at that number by
    Code:
    printf("%u", 448 - 713);
    What you may want to do is change the strncpy to either of these:
    Code:
    strcpy(tmpstr1, chunk.memory);
    Code:
    strncpy(tmpstr1, chunk.memory, 2048); //2048 being the size allocated to tmpstr1

  11. #11
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    but then that defeats the whole point of using strncpy: to remove the last 773 chars from the string.

    EDIT:
    I just tried commenting out that whole section of code and it made no difference - I still get a segfault.

  12. #12
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    strncpy() can do it, but you can't feed it a negative number. If your string in chunk.memory is less than 773 characters then strncpy() is going to break as you are seeing. Try this instead:
    Code:
    if (strlen(chunk.memory) >773)
    {//chunk.memory is bigger than 773 characters
        chunk.memory[strlen(chunk.memory) - 773] = '\0';
    }
    else
    {//Not enough characters, so cut them all off I guess
        chunk.memory[0] = '\0';
    }
    
    strcpy(tmpstr1, chunk.memory);
    You may want to make a copy of the character you overwrote with the '\0' in case you need it later on, but you should get the idea of what I'm suggesting.

    You'll find that many of the C functions have 1 purpose, and trying to use them outside of that purpose will just cause problems. I'd point that at any of the ones that deal with buffers; your comments on the limits of realloc() and the issue with strncpy() should demonstrate what I mean.

  13. #13
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    Quote Originally Posted by Mitch
    I just tried commenting out that whole section of code and it made no difference - I still get a segfault.
    Whats left in the program then? From what you put in the initial post there isn't anything else that should be causing the segmentation fault.

  14. #14
    Unregistered User
    Join Date
    Jan 2005
    Location
    York, UK
    Posts
    8
    Quote Originally Posted by Exile
    Whats left in the program then? From what you put in the initial post there isn't anything else that should be causing the segmentation fault.
    hence my asking here

    i just cant figure it out.
    I've just replaced the
    chunk.memory=&chunk.memory[1];
    line with a new one using memmove, still got it.

    EDIT:
    I read something somewhere about gcc being an arse with write protecting strings sometimes, could it be this? Though I dont see why it would change it just for 1 point in the program.

  15. #15
    Registered User
    Join Date
    Aug 2004
    Posts
    77
    Quote Originally Posted by Mitch
    I just tried commenting out that whole section of code and it made no difference - I still get a segfault.
    Quote Originally Posted by Mitch
    I've just replaced the
    chunk.memory=&chunk.memory[1];
    line with a new one using memmove, still got it.
    What does the code look like now? I'm not sure whats left from the snippet in the original post (if anything at all).

    Quote Originally Posted by Mitch
    I read something somewhere about gcc being an arse with write protecting strings sometimes, could it be this? Though I dont see why it would change it just for 1 point in the program.
    I doubt it. You don't have any constant strings by the sounds of things. The write protected/constant strings are those written in code as "String", for example:
    Code:
    char String[] = "Constant string";

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help understanding a problem
    By dnguyen1022 in forum C++ Programming
    Replies: 2
    Last Post: 04-29-2009, 04:21 PM
  2. Memory problem with Borland C 3.1
    By AZ1699 in forum C Programming
    Replies: 16
    Last Post: 11-16-2007, 11:22 AM
  3. Someone having same problem with Code Block?
    By ofayto in forum C++ Programming
    Replies: 1
    Last Post: 07-12-2007, 08:38 AM
  4. A question related to strcmp
    By meili100 in forum C++ Programming
    Replies: 6
    Last Post: 07-07-2007, 02:51 PM
  5. WS_POPUP, continuation of old problem
    By blurrymadness in forum Windows Programming
    Replies: 1
    Last Post: 04-20-2007, 06:54 PM