Problem with small piece of C code

[kalyan.listsubs]

Hi,

I have the below program which will simply write struct employee to a
file (binary mode). The problem here is empid is writen to the file but
the name (char name[40]) is not written. I am using cygwin (gcc
compiler) and use the following commands to compile & run respectively:

gcc -o emp emp.c

../emp write -> this will create a file emp.out and write employee (Adam
details) to the file
../emp read -> this will reproduce what has been written to the file.

the command ./emp write creates and writes to the file emp.out, but
doesn't includes name so ./emp read doesn't includes name variable in
the employee struct.

Please help me & let me know what is missing or wrong in this piece of
code.

Thank you.

code foe emp.c
==================

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

typedef struct employee_ {
int empid;
char name[40];
} employee;

FILE *fp;

int main(int argc, char **argv)
{
employee *emp = (employee*) malloc(sizeof(employee)+sizeof(40));
if(strcmp(argv[1],"write") == 0)
{
emp->empid = 1;
char *pbuf = "Adam";
strcpy(emp->name,pbuf);
fp=fopen("emp.out","wb+");
fwrite(emp,1,sizeof(emp),fp);
fflush(fp);
fclose(fp);
}
else
{
fp=fopen("emp.out","rb+");
fread(emp,1,sizeof(emp),fp);
fclose(fp);
char *pbuf = (char*)malloc(41);
strcpy(pbuf,emp->name);
printf("%d,%s\n",emp->empid,pbuf);
}
return 0;
}

 

[Ulrich Eckhardt]

typedef struct employee_ {
int empid;
char name[40];
} employee;

FILE *fp;

Why is this a global? As general advise, keep variables as local in scope
as possible, it helps to not loose focus on how and where they are used.

int main(int argc, char **argv)
{
employee *emp = (employee*) malloc(sizeof(employee)+sizeof(40));

1. You should read the FAQ before posting here, it explains about the
dangers and uselessness of casting the returnvalue of malloc().
2. 'sizeof' doesn't need brackets when used with an object, only when used
with a type.
3. 'employee' includes the forty bytes of its 'name' field, no need to
allocate extra.
4. How about checking the returnvalue?
5. Looking at the rest of your program, there is no need at all to
dynamically allocate this struct.

fp=fopen("emp.out","wb+");
fwrite(emp,1,sizeof(emp),fp);

'emp' is a pointer, 'sizeof emp' (useless brackets removed) is the size of
a pointer - probably not what you want.

fflush(fp);
fclose(fp);

You're closing anyway, no need to flush. Might only degrade performance.

fp=fopen("emp.out","rb+");
fread(emp,1,sizeof(emp),fp);
fclose(fp);
char *pbuf = (char*)malloc(41);
strcpy(pbuf,emp->name);
printf("%d,%s\n",emp->empid,pbuf);

Why? Why copy to a temporary buffer? Why allocate such a small, fixed-size
buffer dynamically? Other than that, same problem as above.

Uli

 

[CBFalconer]

I have the below program which will simply write struct employee to a
file (binary mode). The problem here is empid is writen to the file but
the name (char name[40]) is not written. I am using cygwin (gcc
compiler) and use the following commands to compile & run respectively:

gcc -o emp emp.c

./emp write -> this will create a file emp.out and write employee (Adam
details) to the file
./emp read -> this will reproduce what has been written to the file.

the command ./emp write creates and writes to the file emp.out, but
doesn't includes name so ./emp read doesn't includes name variable in
the employee struct.

code foe emp.c
==================

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

typedef struct employee_ {
int empid;
char name[40];
} employee;

FILE *fp;

int main(int argc, char **argv)
{
employee *emp = (employee*) malloc(sizeof(employee)+sizeof(40));
if(strcmp(argv[1],"write") == 0)
{
emp->empid = 1;
char *pbuf = "Adam";
strcpy(emp->name,pbuf);
fp=fopen("emp.out","wb+");
fwrite(emp,1,sizeof(emp),fp);
fflush(fp);
fclose(fp);
}
else
{
fp=fopen("emp.out","rb+");
fread(emp,1,sizeof(emp),fp);
fclose(fp);
char *pbuf = (char*)malloc(41);
strcpy(pbuf,emp->name);
printf("%d,%s\n",emp->empid,pbuf);
}
return 0;
}

To start with, I get the following errors/warnings:

junk.c: In function `main':
junk.c:18: warning: initialization discards qualifiers from pointer
target type
junk.c:18: warning: ISO C89 forbids mixed declarations and code
junk.c:30: warning: ISO C89 forbids mixed declarations and code
junk.c:12: warning: unused parameter `argc'

Now, you have no need for pbuf, because you have declared an
employee such that the structure includes a 40 char field. There
is also no need for fp to be global, and you should NEVER cast the
return from malloc, but you should always test it for success. You
should also test that argv[1] exists before using it, not to
mention the success of a fopen call. In addition, the thing
written (or read) should not be the pointer, but what the pointer
points at. So the size written/read needs correction. So fixing
some of these things we get:

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

typedef struct employee_ {
int empid;
char name[40];
} employee;

int main(int argc, char **argv)
{
employee *emp;
FILE *fp;

if (argc > 1) {
if (emp = malloc(sizeof *emp)) {
if (strcmp(argv[1], "write") == 0) {
emp->empid = 1;
strcpy(emp->name, "Adam");
if (fp = fopen("emp.out", "wb+")) {
fwrite(emp, 1, sizeof(*emp), fp);
fflush(fp);
fclose(fp);
}
}
else {
if (fp = fopen("emp.out", "rb+")) {
fread(emp, 1, sizeof(*emp), fp);
fclose(fp);
printf("%d, %s\n", emp->empid, emp->name);
}
}
}
}
return 0;
}

and it shows signs of working.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

 

[Keith Thompson]

typedef struct employee_ {
int empid;
char name[40];
} employee;

FILE *fp;

Why is this a global? As general advise, keep variables as local in scope
as possible, it helps to not loose focus on how and where they are used.

int main(int argc, char **argv)
{
employee *emp = (employee*) malloc(sizeof(employee)+sizeof(40));

[...]
3. 'employee' includes the forty bytes of its 'name' field, no need to
allocate extra.

And sizeof(40) is just silly. 40 is an expression of type int, so
sizeof(40) is equivalent to sizeof(int).

 

[Peter Shaggy Haywood]

Groovy hepcat CBFalconer was jivin' on Sun, 14 May 2006 09:47:55 -0400
in comp.lang.c.
Re: Problem with small piece of C code's a cool scene! Dig it!

I have the below program which will simply write struct employee to a
file (binary mode). The problem here is empid is writen to the file but
the name (char name[40]) is not written. I am using cygwin (gcc
compiler) and use the following commands to compile & run respectively:

gcc -o emp emp.c

./emp write -> this will create a file emp.out and write employee (Adam
details) to the file
./emp read -> this will reproduce what has been written to the file.

the command ./emp write creates and writes to the file emp.out, but
doesn't includes name so ./emp read doesn't includes name variable in
the employee struct.

code foe emp.c
==================

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

typedef struct employee_ {
int empid;
char name[40];
} employee;

FILE *fp;

int main(int argc, char **argv)
{
employee *emp = (employee*) malloc(sizeof(employee)+sizeof(40));
if(strcmp(argv[1],"write") == 0)
{
emp->empid = 1;
char *pbuf = "Adam";
strcpy(emp->name,pbuf);
fp=fopen("emp.out","wb+");
fwrite(emp,1,sizeof(emp),fp);
fflush(fp);
fclose(fp);
}
else
{
fp=fopen("emp.out","rb+");
fread(emp,1,sizeof(emp),fp);
fclose(fp);
char *pbuf = (char*)malloc(41);
strcpy(pbuf,emp->name);
printf("%d,%s\n",emp->empid,pbuf);
}
return 0;
}

To start with, I get the following errors/warnings:

junk.c: In function `main':
junk.c:18: warning: initialization discards qualifiers from pointer
target type

Presumably that's for the line

char *pbuf = "Adam";

But the diagnostic message is spurious. A string literal has no
qualifiers. It would be nice if they were const qualified. And you
have presumably used some kind of compiler switch or something to make
it so. However, this is an extention.

--

Dig the even newer still, yet more improved, sig!

http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?

 

[CBFalconer]

CBFalconer wrote:
.... snip ...

Presumably that's for the line

char *pbuf = "Adam";

But the diagnostic message is spurious. A string literal has no
qualifiers. It would be nice if they were const qualified. And you
have presumably used some kind of compiler switch or something to
make it so. However, this is an extention.

Yup. I routinely run gcc with -Wwrite-strings, via an alias.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

 

[Keith Thompson]

Yup. I routinely run gcc with -Wwrite-strings, via an alias.

And the resulting warning message is incorrectly worded, but
nevertheless useful.