tags:

views:

153

answers:

5
+2  Q: 

Malloc and scanf

I'm fairly competent in a few scripting languages, but I'm finally forcing myself to learn raw C. I'm just playing around with some basic stuff (I/O right now). How can I allocate heap memory, store a string in the allocated memory, and then spit it back out out? This is what I have right now, how can I make it work correctly?

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
  char *toParseStr = (char*)malloc(10);
  scanf("Enter a string",&toParseStr);
  printf("%s",toParseStr);
  return 0;
}

Currently I'm getting weird output like '8'\'.

+3  A: 

You don't need an & before toParseStr in scanf as it is already a pointer

also call free(toParseStr) afterwards

Sanjay Manohar
Depending on bball's system, it might be necessary to put a "\n" in that printf for things to show up correctly. Also, 10 characters is a REALLY short string.
George
Jerry Coffin
@Jerry It's harmless because the format specifier doesn't specify any arguments, but once he fixes it to have a %s like in your answer it's going to cause a segfault
Michael Mrozek
@Michael: Yes, but it's pointing to only one relatively minor problem where there are many others that are much more serious. In particular, changing that particular point (while leaving the rest of the code unchanged) won't provide any (visible) improvement in its behavior at all.
Jerry Coffin
+8  A: 

You need to give scanf a conversion format so it knows you want to read a string -- right now, you're just displaying whatever garbage happened to be in the memory you allocated. Rather than try to describe all the problems, here's some code that should at least be close to working:

char *toParseStr = malloc(10);
printf("Enter a string: ");
scanf("%9s", toParseStr);
printf("\n%s\n", toParsestr);
/* Edit, added: */ 
free(toParseStr);
return 0;

Edit: In this case, freeing the string doesn't make any real difference, but as others have pointed out, it is a good habit to cultivate nonetheless.

Jerry Coffin
+1 for correctly using the `%9s` in `scanf`.
sarnold
Contrary to some people's belief, flushing `stdout` is *not* necessary to ensure the prompt appears before the input is read, unless the implementation is well and truly broken. For those who really care, see §7.19.3. `stdout` can only be fully buffered if it can be determined *not* to refer to an interactive device.
Jerry Coffin
@Jerry: you're wrong. `stdout` can still be **line-buffered** which means nothing will show up until a newline is printed. POSIX recommends that implementations flush `stdout` and other such line-buffered streams whenever reading, but it's a significant performance hit to scan the open file list for line-buffered streams (especially with threads and locking) and an implementation may choose not to do so for very good reasons. As far as I know, ISO C makes few/no requirements on the buffering semantics. So you **should** flush!
R..
+3  A: 
  char *toParseStr = (char*)malloc(10);
  printf("Enter string here: ");
  scanf("%s",toParseStr);
  printf("%s",toParseStr);
  free(toParseStr);

Firstly, the string in scanf is specifies the input it's going to receive. In order to display a string before accepting keyboard input, use printf as shown.

Secondly, you don't need to dereference toParseStr since it's pointing to a character array of size 10 as you allocated with malloc. If you were using a function which would point it to another memory location, then &toParseStr is required.

For example, suppose you wanted to write a function to allocate memory. Then you'd need &toParseStr since you're changing the contents of the pointer variable (which is an address in memory --- you can see for yourself by printing its contents).

void AllocateString(char ** ptr_string, const int n)
{
    *ptr_string = (char*)malloc(n)
}

As you can see, it accepts char ** ptr_string which reads as a pointer which stores the memory location of a pointer which will store the memory address (after the malloc operation) of the first byte of an allocated block of n bytes (right now it has some garbage memory address since it is uninitialized).

int main(int argc, char *argv[])
{
  char *toParseStr;
  const int n = 10;
  printf("Garbage: %p\n",toParseStr);
  AllocateString(&toParseStr,n);
  printf("Address of the first element of a contiguous array of %d bytes: %p\n",n,toParseStr);

  printf("Enter string here: ");
  scanf("%s",toParseStr);
  printf("%s",toParseStr);
  free(toParseStr);

  return 0;
}

Thirdly, it is recommended to free memory you allocate. Even though this is your whole program, and this memory will be deallocated when the program quits, it's still good practice.

Jacob
+1 for freeing even in a small program. Reminds me of "Little drops make an ocean". ;-)
Praveen S
You should call `fflush(stdout);` between printing the prompt and calling `scanf`. Most implementations will do this for you to be polite, but it's not mandated.
R..
A: 

First, the errors that was keeping your program from working: scanf(3) takes a format-string, just like printf(3), not a string to print for the user. Second, you were passing the address of the pointer toParseStr, rather than the pointer toParseStr.

I also removed the needless cast from your call to malloc(3).

An improvement that your program still needs is to use scanf(3)'s a option to allocate memory for you -- so that some joker putting ten characters into your string doesn't start stomping on unrelated memory. (Yes, C will let someone overwrite almost the entire address space with this program, as written. Giant security flaw. :)

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
  char *toParseStr = malloc(10);
  printf("Enter a short string: ");
  scanf("%s",toParseStr);
  printf("%s\n",toParseStr);
  return 0;
}
sarnold
`scanf` has no `a` option. This is a GNU extension which is not only nonstandard but **CONFLICTS** with ISO C (`%a` is one of the specifiers to read a floating point number!). It should be avoided absolutely.
R..
Thank you; I was unaware this extension clashed with ISO C.
sarnold
+2  A: 

Using scanf() (or fscanf() on data you don't control) with a standard "%s" specifier is a near-certain way to get yourself into trouble with buffer overflows.

The classic example is that it I enter the string "This string is way more than 10 characters" into your program, chaos will ensue, cats and dogs will begin sleeping together and a naked singularity may well appear and consume the Earth (most people just state "undefined behaviour" but I think my description is better).

I actively discourage the use of functions that cannot provide protection. I would urge you (especially as a newcomer to C) to use fgets() to read your input since you can control buffer overflows with it a lot easier, and it's more suited to simple line input than scanf().

Once you have a line, you can then call sscanf() on it to your heart's content which, by the way, you don't need to do in this particular case since you're only getting a raw string anyway.

I would use:

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

#define BUFFSZ 10

int main(int argc, char *argv[]) {
  char *toParseStr = malloc(BUFFSZ+2);
  if (toParseStr == NULL) {
      printf ("Could not allocate memory!\n");
      return 1;
  }
  printf ("Enter a string: ");
  if (fgets (toParseStr, BUFFSZ+2, stdin) == NULL) {
      printf ("\nGot end of file!\n");
      return 1;
  }
  printf("Your string was: %s",toParseStr);
  if (toParseStr[strlen (toParseStr) - 1] != '\n') {
      printf ("\nIn addition, your string was too long!\n");
  }
  free (toParseStr);
  return 0;
}
paxdiablo
+1, though I'd add that while `fgets` does have advantages, `scanf` and `fscanf` *do* have provisions to prevent buffer overflows as well.
Jerry Coffin
That's a good point, @Jerry, though I've rarely seen people use the width specifier with "%s" :-) Since most of my console I/O code tends to have line-based input, %s is unsuitable for getting the white space. However, since your answer is actually right in this case, +1 for you.
paxdiablo
@Paxdiablo: Another interesting possibility is `scanf("%9[^\n]", your_string);` -- line-oriented string input from `scanf`, for whatever that's worth.
Jerry Coffin
@Jerry Coffin: `scanf` and `fscanf` are generally hard to use for other reasons too. IMO it's better for anyone who isn't a C expert to avoid them completely. Anyway, +1 for being the only answer that warns about the potential buffer overflow.
jamesdlin
@Jerry: +1 for the nice `%[` suggestion. So few people know it exists. It's actually useful for implementing a fully portable version of GNU `getline`/`getdelim` on plain ISO C. And if you use `%n` after it, you can even get the read byte count, in case the data read contains embedded null bytes.
R..