-8

This question was asked in my Tally interview question please help me and tell me whats the error in the C code given below. I'd be grateful.

int main()
{
    char *p="Tally";
    strcpy(p,"piyush");
    printf("%s",p);
}
Dan
  • 10,531
  • 2
  • 36
  • 55
Ayush Srivastava
  • 95
  • 11
  • Also, instead of massive downvotes and no explanations, try pointing him in the right direction for once. `p` is not read-only. It just doesn't point to a modifiable memory location. For this to work, you'd have to set it to a modifiable location, e.g. `p = new char[100]` or, to stay with `C`, `p = malloc(100)`. Don't forget error checking though. – arne Oct 18 '13 at 14:26
  • @AlterMann please can you explain in detail. i am someone who is new to programming – Ayush Srivastava Oct 18 '13 at 14:26
  • Good chances are, it's not going to print anything (except perhaps SIGSEGV). – Sergey Kalinichenko Oct 18 '13 at 14:27
  • possible duplicate of [Is modification of string literals undefined behaviour according to the C89 standard?](http://stackoverflow.com/questions/10001202/is-modification-of-string-literals-undefined-behaviour-according-to-the-c89-stan) – Sergey Kalinichenko Oct 18 '13 at 14:32
  • @arne `p = malloc(100);` is a pretty darn bad piece of code to give to a begginer. This makes no sense to write anywhere. The correct way of doing this is by writing `char *p = malloc(sizeof(*p) * strlen("Tally")); strcpy(p, "Tally");`. Or, in short `char *p = strdup("Tally");` – Eregrith Oct 18 '13 at 14:48

7 Answers7

3

There are several "errors" in this code (whatever was supposed to be meant by "error").

The primary issue is that the code calls undeclared functions strcpy and printf. Formally, this is a compile error in modern C. And in case of printf this is undefined behavior in pre-C99 versions of C.

And if we fix that issue, then the strcpy call will make an attempt to modify a string literal. String literals are not modifiable. Such modification attempt will cause undefined behavior.

Because of the above UB, it is impossible to say what is passed to printf, but it looks like printf call was meant to perform output to a text stream without finishing the last line with newline character. It is implementation-defined whether such newline character is required.

Finally, although it is not an "error", one can argue that const char * pointers should normally be used for pointing at string literals.

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • +1, but I don't get the newline point... since when it's mandatory to terminate the output with a newline? If it's about line buffering, the streams are guaranteed to be closed at normal program termination (and `fclose` implies flushing), so that shouldn't be a problem... – Matteo Italia Oct 18 '13 at 14:52
  • 1
    @Matteo Italia: Well, the specification of text streams in the standard explicitly states *"Whether the last line requires a terminating new-line character is implementation-defined"*. It uses the word "requires", but it does not elaborate on what should happen if this requirement is violated. I believe it is supposed to mean more that just some late stream flushing. I'd guess that it implies that on some platforms a text stream *shall* have newline at the end of each line to be considered valid. – AnT stands with Russia Oct 18 '13 at 14:55
  • Thank you, I agree with your interpretation; I suppose that's for some archaic platforms where text files were stored as a list of lines, and a newline was required to allow the CRT to split the stream in lines. (For future reference, that quotation is at §7.19.2, ¶2). – Matteo Italia Oct 18 '13 at 15:00
1

p is a pointer to the string literal "Tally". You cannot overwrite a literal.

lolando
  • 1,721
  • 1
  • 18
  • 22
1

In line 3 you are storing a pointer to a string literal, then you try to overwrite its content with that strcpy; problem is, string literals are read-only (it's UB if you try to write on them, and on modern platforms it typically results in a crash).

If you want a writable string, you have to allocate a local buffer (wide enough for any data you want to store in it).

Still, these are the basis of string handling in C, I strongly suggest you to revise these arguments on your C book before going further.

Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
1

p is a pointer to a five character string literal. piyush is six characters long. This will overflow the space allocated for *p, and even if it didn't, *p can not be modified due to the way it was declared.

Dan
  • 10,531
  • 2
  • 36
  • 55
  • 1
    `p` is declared `char *` which doesn't prohibit writing per se. It just doesn't point to a writable location. – arne Oct 18 '13 at 14:27
  • Correct - the data pointed to by `p` cannot be modified because it's a literal, which is why I said `*p` cannot be modified. `p` itself could be modified, he could do `p = "piyush"` or even `p = malloc(7*sizeof(char)); strcpy(p, "piyush")`, but that isn't what he wants I think. – Dan Oct 18 '13 at 14:29
  • Oh dear. I overread the `*` on `p` in your last sentence. Still, it's not all too clear what the actual problem is. – arne Oct 18 '13 at 14:30
1

"Tally" is in the read only section of the executable. You are trying to change it, It balks and therefore fails. What is the problem

Ed Heal
  • 59,252
  • 17
  • 87
  • 127
0

this line is wrong

char *p="Tally";

the pointer has no adres where it has to write to you can do for example this

char t;
char *p;
p = &t;
*p='a';

o and in a char you cant put a string

edit a nice link to look at http://www.physics.drexel.edu/courses/Comp_Phys/General/C_basics/#pointers

Robert Stevens
  • 488
  • 1
  • 7
  • 21
0

Answer to your interview question - Here are the errors with this code :

1. No libraries included, hence printf and strcpy are undeclared functions

2. int main() has no return in the code. return 0; is required // gcc will throw a warning when -Wall flag is set

3. p is a string literal. Which can't be overwritten.

EDIT:

For those of you who say return is not required: Here is the result when compiling with gcc with -Wall flag set:

Notra:Desktop Sukhvir$ gcc -Werror -Wall -g -o try try.c
cc1: warnings being treated as errors
try.c: In function ‘main’:
try.c:19: warning: control reaches end of non-void function

Granted compiler will overlook this if -Wall isn't set, but it is still good practice to include it. @AndreT the very fact that gcc throws up a warning is the hint that it is meant to be there. Sure code can still compile without it .. but that doesn't make it good practice

Edit 2:

when gcc is forced to use C99 standards, it compiles without warnings. So please disregard point 2 of my post.

sukhvir
  • 5,265
  • 6
  • 41
  • 43
  • `return` is not required in `main`. C99 and later will automatically return zero from `main`. – AnT stands with Russia Oct 18 '13 at 14:34
  • @AndreyT code won't compile is -Wall flag is set in gcc. Also its good practice to include return a value – sukhvir Oct 18 '13 at 14:36
  • return is not required in main. – Ayush Srivastava Oct 18 '13 at 14:37
  • @sukhvir: I'm not sure what you mean by "won't compile". A compliant C99 (and later) C compiler is required to compile this code. If your compiler refuses to do it, there must be something wrong with the compiler or with the setup. What version of GCC are you using? Are you by any chance compiling in C89/90 mode? – AnT stands with Russia Oct 18 '13 at 14:39
  • GCC will issue this warning in -Wall mode only if used in legacy C89/90 mode. So, if you want to target your answer at the legacy versions of the language, you have to note it specifically. In the current C no such issue exists and no warnings are generated by GCC. Firstly, if you want to use GCC for compiling C code, `-std=c99` setting or higher is required. Secondly, if you want to judge the validity of code by GCC wanings, you have to make a huge effort of configuring GCC properly. In default mode GCC is a humorous zoo of nonsensical feedback. – AnT stands with Russia Oct 18 '13 at 14:44
  • @AndreyT I wasn't aware that this will not be a problem in C99. Could you double check on your c99 compatible gcc. – sukhvir Oct 18 '13 at 14:48
  • @sukhvir: Chances are, your GCC is already "C99 compatible". You just have to remember to specify `-std=c99` explicitly. Again: in default mode GCC is not C. It is not even C89/90. In order to make GCC comply to any specification of C (be that the old one or the new ones), you have too whip it into shape by specifying several options, like (`-std=...`, `-ansi`, `-pedantic-errors` etc.) – AnT stands with Russia Oct 18 '13 at 14:50
  • dammit .. i just tried compiling with -std=c99 .. you are correct .. it compiles without warning. Having said that .. would you consider it a good coding practice to not use returns – sukhvir Oct 18 '13 at 14:52