C++ error of buffer after strcpy

Question

This is piece of my code

    std::string s;  
    getline(cin, s);

    std::cin.clear();
    s.erase(std::remove_if(s.begin(), s.end(), my_predicate), s.end());
    char *str1  = (char*)malloc(sizeof(char)*(s.size()+1));
    const int length = s.length();
    for(int i=0; i < length; ++i)
    {
        s[i] = std::tolower(s[i]);
    }
    strncpy(str1,s.c_str(), s.length()+1);

    printf("\n");

The problem is if I do not print \n the code crashes. I have tried memcpy and strdup also but to no avail. Is there any error or any way to get rid of this ????

`char *str1 = = (char*)malloc(sizeof(char)*(s.size()+1));` does that even compile with a double = ? — mathematician1975, Sep 21 '12 at 08:49
if you really use **C++**, as your tags indicate, then switch to std::string in all cases instead of using `char *`! This will usually remedy all invalid accesses due to invalid pointer arithmetics or wrong indexing... What do you actually want to achieve? Variable `t1` is completeley unused, by the way, and `s` already seems to contain the proper end result (the lowercase string)! So no need for `str1` and `t1`!!! — codeling, Sep 21 '12 at 08:55
@mathematician1975 - that was an edit issue, sorry for that.... — Bhavya Agarwal, Sep 21 '12 at 08:57
@RandolphCarter - those variables are used further in the code... — Bhavya Agarwal, Sep 21 '12 at 08:57
but why use `char *`? If you have to use API functions where they are needed, convert to `char *` at the last possible moment via `c_str()`! — codeling, Sep 21 '12 at 08:58
length is still used in the for loop, you shouldn't have removed its definition. — Kit Fisto, Sep 21 '12 at 09:00
@cschwan removed t1 i tried new but still same error. As you pinted out I needed to use an API function and didn't have any alternative to char* Is there any other way to copy the contents of s to str1 — Bhavya Agarwal, Sep 21 '12 at 09:03
have a look here: http://stackoverflow.com/questions/347949/convert-stdstring-to-const-char-or-char — codeling, Sep 21 '12 at 09:04
usa calloc to allocate the memory, and DO NOT copy s.length() + 1 bytes in strncpy(str1,s.c_str(), s.length()+1); but s.length() only — Ferenc Deak, Sep 21 '12 at 09:05
@KitFisto - got nervous after downvotes.. thanks for pointing out.. :) — Bhavya Agarwal, Sep 21 '12 at 09:15
@fritzone: This advice would prevent the trailing 0 from being copied. — Kit Fisto, Sep 21 '12 at 09:18
tried all of them but still the error persists code works only after \n is printed — Bhavya Agarwal, Sep 21 '12 at 09:45
@Bhavya Agarwal : please explain the background of your problem a little more deeply before delving straight into the issue. FOr example, what is the code about? or what are you actually trying to achieve. That would help you get much better answers, and *hopefully* more upvotes than downers... :D — kumarharsh, Sep 21 '12 at 10:59
@KitFisto: I concur. most people don't know that hitting the length-specifier in chars-copied using strncpy will NOT set the terminating-0 unless it is included in the 'n'. Similarly, most don't know it backfills the target buffer with 0's if the length-specifier exceeds the length of the source string. POSIX specs ftw. — WhozCraig, Sep 21 '12 at 14:37

Kit Fisto · Answer 1 · 2012-09-21T09:12:36.280

0

While I do not think your code is perfect, I cannot see why it should crash. Since you say, printf("\n") prevents the crash I suppose the heap is already corrupted. Perhaps show the code from myPredicate or test the given snippet stand-alone to find the reason.

edited Sep 21 '12 at 09:12

answered Sep 21 '12 at 09:04

Kit Fisto

4,385
5
26
43

bool my_predicate(char c) { return !(isalpha(c) || c==' '); } – Bhavya Agarwal Sep 21 '12 at 10:01

score 0 · Answer 2 · edited May 23 '17 at 12:12

0

If you absolutely need to have a writable copy of the std::string s, check this answer to a related question; instead of

char *str1  = (char*)malloc(sizeof(char)*(s.size()+1));
strncpy(str1,s.c_str(), s.length()+1);

basically, do this:

boost::scoped_array<char> writable(new char[s.size() + 1]);
std::copy(s.begin(), s.end(), writable.get());
writable[s.size()] = '\0'; 

// access char * via writable.get()!

This comes with the additional benefit of not having to worry whether to free any memory afterwards!

edited May 23 '17 at 12:12

Community

1
1

answered Sep 21 '12 at 09:08

codeling

11,056
4
42
71

2

You could just use a `std::vector writable( s.begin(), s.end() );` and then access the `char*` via `&writable[0]`. – Frerich Raabe Sep 21 '12 at 09:11

C++ error of buffer after strcpy

2 Answers2