2
char* s1;
strcpy(s1,"smilehihi");
s1[6] = 'a';

When I compile, VS do not have any errors. But in the runtime, my code makes mistake. I think I do not really understand about strcpy

M.M
  • 138,810
  • 21
  • 208
  • 365
Steve Andel
  • 21
  • 3
  • 3
    you haven't allocated any space for s1 – dangee1705 Apr 21 '20 at 02:07
  • 1
    maybe you want to use strdup? – dangee1705 Apr 21 '20 at 02:07
  • 1
    `char* s1;` is an uninitialized pointer. – drescherjm Apr 21 '20 at 02:18
  • 3
    This is really a C question. C++ uses `std::string`. – tadman Apr 21 '20 at 02:25
  • 1
    `strcpy()` ASSUMES that `s1` points at the first element of an array, so that all of the characters in `"smilehihi"` - including the terminating nul character, so ten characters - can be copied. The behaviour is undefined if that assumption is untrue. In your case, `s1` is uninitialised - so even the act of passing its value to `strcpy()` gives undefined behaviour, and it certainly doesn't point at any array that can be copied to. Either pass an array that is known to have 10 or more characters, or initialise `s1` so it points to the first element of such a buffer. – Peter Apr 21 '20 at 02:27
  • @tadman `char*` and `strcpy` are also parts of C++. OP has clearly indicated they are using C++ – M.M Apr 21 '20 at 02:31
  • @M.M Can we tamp down the pedantic comments for new people? This is plain old C code with plain old C problems. Unless this is `std::strcpy`, which it does not appear to be, this is C `strcpy`. – tadman Apr 21 '20 at 02:32
  • @M.M I don't assume they're lying, I assume they're confused, and that's something that happens a lot because C++ doesn't really blink when ingesting C code like this. I changed the tag because this is a C question and anyone looking for things to answer about C could weigh in. – tadman Apr 21 '20 at 02:35
  • @tadman They said twice (title and tag) that they are using C++, therefore this is a C++ question, end of story . You seem in denial of the fact that C++ has pointers and strcpy. – M.M Apr 21 '20 at 02:37
  • 3
    *When I compile, VS do not have any errors* -- The compiler only checks for syntax errors, not if your program is logically correct. – PaulMcKenzie Apr 21 '20 at 02:37
  • 2
    @tadman - I agree with M.M on this one. When a beginner is learning C++, and uses C constructs that ARE valid in C++, then telling them they are using C - a language, that as far as they know, they are not learning - doesn't help all that much. Better to leave the tags alone, address the question, and then - as appropriate - encourage use of alternatives (like using `std::string`) that are preferable in C++. – Peter Apr 21 '20 at 02:55
  • @Peter If you want more "C/C++" questions this is how you encourage that. – tadman Apr 21 '20 at 03:19
  • @tadman - Not really. "C/C++" questions tend to be deliberately asked in that manner - the person asking the question knows they are doing it. Getting into religious language debates with someone who believes they are learning C++ (or C) but have used something common to both languages, doesn't address that.\ – Peter Apr 21 '20 at 04:07
  • @tadman check the tag wiki, it clearly says that if the person is using a C++ compiler then the question should be tagged as C++ . And gives guidelines about when to dual tag (this is not such a case) – M.M Apr 21 '20 at 04:13
  • "When I compile, VS do not have any errors" - but even with warning level set all the way down to 1 (which no sane person would do) it does warn "warning C4700: uninitialized local variable 's1' used". – Allison Lock Apr 21 '20 at 09:13

4 Answers4

2

The main issue here is not the strcpy() function but the fact that you don't allocate any memory for the string itself. If I were you, I would do something like

char* s1=(char*)malloc(SIZE); // the SIZE is the predefined maximum size of your string
strcpy(s1,"smilehihi");
s1[6] = 'a';

Edit:

Just as an advice, consider using stpncpy(). It helps to avoid buffer overflow, and, in your case, will help you avoid exceeding the maximum size of char*

char * stpncpy(char * dst, const char * src, size_t len);
Alex.Kh
  • 572
  • 7
  • 15
1

The problem is that you have not allocated any space for what you wish to store in s1: "smilehihi". You declare s1 as a pointer variable, but it needs something to point at. You can allocate space by using the new operator.

char* s1 = new char[stringLength + 1]; //stringLength = length of string stored
                                       // + 1 to hold null terminator character
strcpy(s1, "smilehihi");
s1[6] = 'a';

You have to declare #define _CRT_SECURE_NO_WARNINGS at the top of your main file to avoid an error during compilation due to strcpy() being deprecated.

ElSnowman
  • 51
  • 4
  • For your particular case, `smilehihi` is 9 characters, plus a null terminator would become 10 characters. Replace `stringLength + 1` with 10 and you should be good. Also make sure to `delete s1;` at the end of your program to unallocate the memory you allocated with `new`. – ElSnowman Apr 21 '20 at 03:10
  • you need to use `delete[]` for memory allocated with `new[]` – Remy Lebeau Apr 21 '20 at 07:07
1

You need to allocate variable first by malloc() or by using the keyword new . Also deallocate the memory at the end

Anupam Haldkar
  • 985
  • 13
  • 15
1

At first you should allocate char* s1.

char *s1 = new char[9]; // C++ version

or you can you use C version:

char *s1 = (char*)malloc(9);

Then you can use following code:

strcpy(s1, "smilehihi");
s1[6] = 'a';
Sina Raoufi
  • 54
  • 2
  • 6