views:

134

answers:

5

I am using Linux. I am trying to write a program in c that will print a string backward. Here is my code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
int main (){
    char string[100];
    printf ("Enter string:\n");
    gets (string);
    int length = strlen (string)-1;
    for (length = length; length>=0; length--){
        puts (string[length]);
    }
}

And here is the error:

a.c:10: warning: passing argument 1 of ‘puts’ makes pointer from integer without a cast
/usr/include/stdio.h:668: note: expected ‘const char *’ but argument is of type ‘char’
/tmp/cc5rpeG7.o: In function `main':
a.c:(.text+0x29): warning: the `gets' function is dangerous and should not be used.

What should I do?

A: 

You should use putchar instead of puts

So this loop:

for (length = length; length>=0; length--){
    puts (string[length]);
}

Will be:

for (length = length; length>=0; length--){
    putchar (string[length]);
}

putchar will take a single char as a parameter and print it to stdout, which is what you want. puts, on the other hand, will print the whole string to stdout. So when you pass a single char to a function that expects a whole string (char array, NULL terminated string), compiler gets confused.

Pablo Santa Cruz
+11  A: 

Forget that the function gets() exists - it is lethal. Use fgets() instead (but note that it does not remove the newline at the end of the line).

You want to put a single character at a time: use putchar() to write it to stdout. Don't forget to add a newline to the output after the loop.

Also, for (length = length; length >= 0; length--) is not idiomatic C. Use one of:

  • for ( ; length >= 0; length--)
  • for (length = strlen(string) - 1; length >= 0; length--)
  • for (int length = strlen(string) - 1; length >= 0; length--)

The last alternative uses a feature added to C99 (which was available in C++ long before).

Also, we could debate whether length is the appropriate name for the variable. It would be better renamed as i or pos or something similar because, although it is initialized to the length of the input, it is actually used as an array index, not as the length of anything.

Subjective: Don't put a space between the name of a function and its parameter list. The founding fathers of C don't do that - neither should you.


Why is gets() lethal?

The first Internet worm - the Morris worm from 1988 - exploited the fingerd program that used gets() instead of fgets(). Since then, numerous programs have been crashed because they used gets() and not fgets() or another alternative.

The fundamental problem is that gets() does not know how much space is available to store the data it reads. This leads to 'buffer overflows', a term which can be searched for in your favourite search engine that will return an enormous number of entries.

If someone types 150 characters of input to the example program, then gets() will store 150 characters in the array which has length 100. This never leads to happiness - it usually leads to a core dump, but with carefully chosen inputs - often generated by a Perl or Python script - you can probably get the program to execute arbitrary other code. This really matters if the program will ever be run by a user with 'elevated privileges'.

Incidentally, gets() is likely to be removed from the Standard C library in the next release (C1x - see n1494 from WG14). It won't vanish from actual C libraries for a long time yet (20 years?), but it should be replaced with this implementation (or something similar):

#undef NDEBUG
#include <assert.h>
char *gets(char *buffer)
{
    assert("Probability of using gets() safely" == 0);
}

One other minor detail, discussed in part under the comments to the main question.

The code shown is clearly for C99; the declaration of length part way through the function is invalid in C89. Given that, it is 'OK' for the main() function not to explicitly return a value, because the C99 standard follows the lead of the C++ standard and allows you to omit the return from main() and the effect is the same as return(0); or return 0; at the end.

As such, the program in this question cannot strictly be faulted for not having a return at the end. However, I regard that as one of the more peculiar standardizing decisions, and would much prefer it if the standards had left that provision out - or done something more radical like allowing the ubiquitous but erroneous void main() observing that when control returns from that, the result is that a success status is returned to the environment. It isn't worth fighting to get that aspect of the standard changed - sadly - but as a personal style decision, I don't take advantage of the licence granted to omit the final return from main(). If the code has to work with C89 compilers, it should have the explicit return 0; at the end (but then the declaration of length has to be fixed too).

Jonathan Leffler
+1 for `gets` warning.
Philip Potter
+1 "`gets` is lethal"
bstpierre
Will be +1, once you explain to him why it is lethal. I like to make people work ;).
Stephen
Bingo, +1. Well done. :)
Stephen
A: 

Use putc or putchar, as puts is specified to take a char* and you are feeding it a char.

Vatine
+1  A: 

You can also use recursion to do it. I think it looks nicer then when using a loop.

Just call the method with your string, and before printing the char in the method, call the method again with the same string, minus the first char.

This will print out you string in reversed order.

Cid54
This works if you're not tight on memory and have small strings. You will get an explosion in stack and memory usage for any non trivial size strings.
bot403
A: 

First:

NEVER NEVER NEVER NEVER NEVER use gets(); it will introduce a point of failure in your code. There's no way to tell gets() how big the target buffer is, so if you pass a buffer sized to hold 10 characters and there's 100 characters in the input stream, gets() will happily store those extra 90 characters in the memory beyond the end of your buffer, potentially clobbering something important. Buffer overruns are an easy malware exploit; the Morris worm specifically exploited a gets() call in sendmail.

Use fgets() instead; it allows you to specify the maximum number of characters to read from the input stream. However, unlike gets(), fgets() will save the terminating newline character to the buffer if there's room for it, so you have to account for that:

char string[100]; 
char *newline;
printf("Enter a string: ");
fflush(stdout);
fgets(string, sizeof string, stdin);
newline = strchr(buffer, '\n');      // search for the newline character
if (newline)                         // if it's present
  *newline = 0;                      // set it to zero

Now that's out of the way...

Your error is coming from the fact that puts() expects an argument of type char *, but you're passing an argument of type char, hence the "pointer from integer without cast" message (char is an integral type). To write a single character to stdout, use putchar() or fputc().

John Bode