malloc for char pointers and printf

Question

I have a file which looks like following:

 ATOM          HIS
 ATOM          TRP
 ATOM          PHE

I want to print the first column, following is my C-code:

#include<stdio.h>
#includ<stdlib.h>
void main
{
     FILE *fp;
     fp=fopen("xyz","r");
     char *atm,*res;
     char buff[200];
     while(fgets(buff,sizeof (buff),fp)!=NULL){
                    i++;
     }
     rewind(fp);
     atm=(char*)malloc(i * sizeof (char*));
     res=(char*)malloc(i * sizeof (char*));
     while(fgets(buff,sizeof (buff),fp)!=NULL){
               fscanf(fp,"%s %s",&atm[i],&res[i]);
               i++;
     }
     for(j=0;j<i;j++){
             printf("%s\n",atm);
     }

I would expect the following output:

ATOM
ATOM
ATOM

But it doesn't compile and says that:

 warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘int’

hence in printf statement I have added & to atm (i.e &atm instead of atm). In this case the code compiles well but gives the following output:

AAAAAAAAAAAAAAAATOM AAAAAAAAAATOM AAAAAAAATOM

I will appreciate any suggestion regarding this.

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Jun 22 '16 at 10:08
@SouravGhosh : I didn't get it. My problem is bit different here. Instead of char/string if I have floats in the column and I do the same operation. It works fine. without errors in compilation or printf. — PythonNoob, Jun 22 '16 at 10:13
It this the actual code or something you made up for posting on SO? Because the compiler error doesn't match the posted code. Also, the obvious compiler errors in this code (`void main`) makes me think this is some artificial stuff you just made up, and not the real code. — Lundin, Jun 22 '16 at 10:18
@Lundin: I didn't actually copy paste it but I wrote it seeing the terminal. There might be some typos. But I guess I am looking for more substantial error than missing braces after "void main". — PythonNoob, Jun 22 '16 at 10:23
@PythonNoob: then *copy and paste* the code, don't type it over. How can anyone help you find actual errors when we can't see what other typos you made? — Martijn Pieters, Jun 22 '16 at 10:29
[Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) — phuclv, Jun 22 '16 at 10:30
@MartijnPieters If I was well versed in C, I wouldn't care for the typos of the person asking question. Especially if the person has mentioned that he has typed the code and what he is looking for is not the errors from the typos but the actual use of malloc, pointers etc. Anyways, that was rude. — PythonNoob, Jun 22 '16 at 10:37
You need a 2d array to hold the results, see possible solution http://www.tutorialspoint.com/compile_c_online.php?PID=0Bw_CjBb95KQMZEpKSndTV3ZHQVU — kwarnke, Jun 22 '16 at 10:39

Sourav Ghosh · Answer 1 · 2016-06-22T10:17:53.630

First of all,

 atm=(char*)malloc(sizeof (char*));

does not do what you think it does. It only allocates a memory enough o hold a char *, that's also wrongly.

You need to allocate a size of n * sizeof(char), where, n == number of chars. Now, sizeof(char) == 1 being guaranteed by C standard, your statement can be reduced to

atm = malloc(requiredsize);
res= malloc(requiredsize);

After that, you should check for the success of malloc() call to prevent UB from accessing NULL pointer.

But then, you'll be overwriting atm and res in every call to fgets() and fscanf(). You seem to need an array of pointers, not a simple pointer to get the job done.

Thanks for pointing that out. I am multiplying which I forgot to mention in the code. Anyways the problem still persists. — PythonNoob, Jun 22 '16 at 10:16

davmac · Answer 2 · 2016-06-22T10:23:34.863

1

If atm and res are pointers to arrays of char *, which seems like the intention, then they should be declared as such:

char **atm,**res;

You then need to allocate storage for each string, as well as the array as a whole:

 while(fgets(buff,sizeof (buff),fp)!=NULL){
     atm[i] = malloc(200); // choose a suitable maximum size
     res[i] = malloc(200);
     fscanf(fp,"%s %s",atm[i],res[i]);
     i++;
 }

This much more closely matches your intention - I think.

It's not particularly good practice, however. Using fscanf with %s, there is no guarantee that the string you read won't be longer than what you've allocated (I think you can use a width specifier to limit what it reads, eg %100s, but that limits how much is read as well as how much is stored). You should also check the return from malloc in each case to make sure it was successful.

edited Jun 22 '16 at 10:23

answered Jun 22 '16 at 10:18

davmac

20,150
1
40
68

Thannks thats helpful. So what should be used in place of `fscanf` ? – PythonNoob Jun 22 '16 at 10:28
1

@PythonNoob you would think that C might include a convient way to read variable-length strings from a file, but that is not the case. You can use `getline` if it is available on your platform (and then munge the result into two separate strings). TBH this deserves a separate question. – davmac Jun 22 '16 at 10:35
I tried the code you suggested and it didnt work. No compilation errors but gives "segmentation fault" upon execution. – PythonNoob Jun 23 '16 at 02:06
@PythonNoob well, there are bugs in your code; quite a few. But you say you didn't paste the exact code; I don't know which are typos and which are real. You're not resetting the value of `i` before the second `while` loop. You're using `fgets` and then `fscanf`, which doesn't make sense; perhaps you should use `sscanf` to scan the buffer that you've already read via `fgets`. Seriously, ask another question, and post your _exact_ code. – davmac Jun 23 '16 at 08:11

malloc for char pointers and printf

2 Answers2