3

This was the question asked to me in an interview in c:

#include<stdio.h>
void main(){
  char *ch;
  ch=fun1();
  printf(ch);
}

fun1(){
  char *arr[100];
  strcpy(arr,"name");
  return arr;
}

I was given the above program and was asked to figure out the problems in the above code. below was my answer

  • function declaration is wrong.the return type should be char **
  • syntax of printf is wrong
  • arr scope is limited to the function fun1

then

Interviewer : what would be your solution to the problem?

Me: you need to make the arr variable as global and fix the remaining issues mentioned above.

Interviewer: Dont you think global variables are dangerous?

Me: Yes ofcourse,since we cannot say where it is being accessed in which functions and sometimes it gets almost impossible to find which function has changed the value

Ineterviewer :give me a solution without a global variable

Me:????

what would be your solution for this? Could anybody pls point out the errors that i have made !!

TechZen
  • 63,819
  • 15
  • 116
  • 144
Vijay
  • 59,537
  • 86
  • 209
  • 308

6 Answers6

13

My solution could be

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

/* copies "name" to `dst`.
** The caller is responsible for making sure
** `dst` has enough space */
char *fun1(char *dst) {
  strcpy(dst, "name");
  return dst;
}

int main(void) {
  char ch[5];
  printf("%s\n", fun1(ch));
  return 0;
}

Here's just what I found about the snippet ...

#include<stdio.h>

I'd add a space before the header: #include <stdio.h>; but that's just for looks

void main()

WRONG! main returns an int. ALWAYS!
And it should be one of: int main(void) or int main(int argc, char **argv) if you need parameters

{
char *ch;
ch=fun1();

No prototype for fun1 in scope. This makes the compiler assume the function returns a value of type int and the statement tries to assign that value to an object of type char* which is illegal: the compiler must issue a diagnostic here.

printf(ch);
}

fun1()

In C99, it is mandatory to specify a return type; in C89, int is assumed. Also this function definition really should also be a prototype (in both C89 and C99) and specifiy the number and types of parameters (or void if it takes none).

{
char *arr[100];

Why do you need an array of 100 pointers? ???

strcpy(arr,"name");

Oops ... no prototype for strcpy() in scope. Also, assuming the strcpy is the one declared in <string.h>, the 1st parameter should be a char* not a char **

return arr;

arr ceases to exist right after the function returns. Its address (the array decays to the address of its first element in this context) is unusable in the calling code.

}
pmg
  • 98,431
  • 10
  • 118
  • 189
  • Well, adding void to mark an empty argument list is completely optional. Most people don't bother, since it adds no information. – jforberg Jul 09 '11 at 16:52
  • @jforberg: not in C. The absence of `void` makes it **not** a prototype. The compiler cannot assume a function without a prototype takes no arguments. (I believe it is different in C++). – pmg Jul 09 '11 at 16:55
  • 1
    It adds the information that you really mean "void" and haven't just forgotten to include the parameters you really meant. Remember that code is written for programmer to read too, and clear, explicit code means fewer mistakes. – Taryn East Jul 09 '11 at 16:58
  • `strcpy(dst, "name");` - so you're trusting callers to guess how big the buffer has to be? – Georg Fritzsche Jul 09 '11 at 17:02
  • @pmg: Why would you want to prototype main()? @Taryn: Well, it's a matter of style. – jforberg Jul 09 '11 at 17:04
  • 1
    Yes, it's a matter of Good Style. One element of Good Style is "make errors obvious so it's more likely that you can spot them" - in this case, being explicit with parameters makes it obvious that you purposely chose to make this a void function - in that way - if you ever did just forget to add params (ie made an error)... it is obvious... because it's different to every other function definition in your file. – Taryn East Jul 09 '11 at 17:06
  • 1
    You claimed it added no information. I counter that it adds precisely the information that "this was done on purpose". ;) – Taryn East Jul 09 '11 at 17:07
  • Example of [prototypes in action](http://codepad.org/EtVX9vzV) LOL – pmg Jul 09 '11 at 17:08
  • @Georg: Yes, I trust the users! BUT (big BUT) I missed the documentation ... added to the code ... thanks – pmg Jul 09 '11 at 17:09
  • You could just allocate space inside the function taking in a pointer to avoid callers to have to know the size as Georg mentioned. Also why are your returning the char pointer in this case, can we just make it a void function since the function is changing the value itself. – bjackfly Oct 17 '13 at 21:06
2

Either a static char arr[100] inside thefunction (this would lead to problems, because it would render it non reentrant) or you should malloc() the array and return its pointer (you should free it later in the caller function).

Don't wait for that company to call back ;)

Vinicius Kamakura
  • 7,299
  • 1
  • 25
  • 42
1
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
void fun1();

int main()
{
char *ch = malloc(sizeof(char) *100);
fun1(ch);
printf("%s\n", ch);
free(ch);
}

fun1 (char* ch)
{
    strcpy(ch,"name");
}

This is the best solution imo. There are obviously other ways to do it. You could declare char arr[100] and pass that in, then set ch = arr, but this basically cuts out the middleman.

Although I suppose this changes the problem format, so I'm not sure if that's allowed. If you have to maintain the structure, then static char arr[100] or malloc() in the function is the way to go.

BlackJack
  • 2,726
  • 1
  • 18
  • 25
  • why dont you declare ch as a local variable instead of using malloc in main? also I think to remember, that the standard of main looks something like "int main(void)" – Christian Goltz Jul 09 '11 at 16:46
  • You could do that too. I just scratched something together fast. int main() is an acceptable alternative. – BlackJack Jul 09 '11 at 16:57
  • @Christian: main(void) is simply a stylistic variation, like putting braces on a line of their own. The (void) adds no information, so is superfluous IMO. – jforberg Jul 09 '11 at 17:02
  • I just found a thread about this and the discussion at http://stackoverflow.com/questions/693788/c-void-arguments points out, that the version with explicit (void) may have some benefits in C. – Christian Goltz Jul 09 '11 at 17:10
  • @Christian: I'm not saying it's wrong; I'm saying it's a matter of taste. Your arguments have merit, but the usage of (void) is not strictly relevant to the question. – jforberg Jul 09 '11 at 17:17
0

I see at least the following additional errors:

  1. The return type of main() should be int, if you're following modern standards.
  2. The puts() function should be used instead of printf(), since no formatting is performed.
  3. The variable arr should probably have type char*, not char** (a single string vs. and array of 100 strings.)
  4. fun1() is called before it is declared & defined, which is an error. In a real project, you would have a forward-declaration in a header file which accompanies you program code.
  5. The program, in its current form, does nothing at all.

You should start by asking your interviewer: "What is the program supposed to do?" before you try to fix it, IMO. If the point is to output "name" to std output, the following code suffices, and is much simpler (that is, better):

#include<stdio.h>
int main() {
    puts("name");
    return 0;
}
jforberg
  • 5,798
  • 3
  • 25
  • 45
-1

static declaration of arr in function

static char arr[100];

this way its still local to function, and you can return a valid pointer to it.

btw, fun1 return type should be char*, not char**

John Carter
  • 50,377
  • 25
  • 100
  • 137
Ulterior
  • 2,378
  • 3
  • 26
  • 56
-1

Hence the string returned by the function fun1 is fixed, func1 can return a const char* type. The string "name" will be stored in read only area and its pointer will be returned.

#include<stdio.h>
int main(){
  const char *ch;
  ch=fun1();
  printf("%s", ch);
  return 0;
}
const char *fun1(){
  const static char* arr[] = "name";
  return arr;
}

You will be able to write as below:

const char *fun1(){
  return "name";
}
kamae
  • 1,576
  • 1
  • 12
  • 19
  • Or just remove the darn function altogether... The program still isn't good/valid C because main() doesn't return int. And what is the type of arr? – jforberg Jul 09 '11 at 21:46