Unix Programming - problem - actual code without error

Author

problem - actual code without error

friend_05

2005-09-27, 5:55 pm

here is my actual code.
only my first data is written properly in file. after first data other
data are written randomly in file. i thinl there is problem of buffer
overflow but i am not able solve the problem.
this code has no errors

header file
inventory.h

#define NAME_MAX 60
#define NUMBER_DIGITS 6
typedef struct stru
{
char ch[NAME_MAX];
char number[NUMBER_DIGITS];
float price;
int seats;
int first, other;
} st_t;

extern int adddata(st_t*);

mainprogram.c

#include<stdio.h>
#include"inventory.h"
#include<fcntl.h>
#include<unistd.h>
#include<stdlib.h>
int main()
{
int fd,a,c;
st_t st;
void display();
void a_delete();

char path[]="records";
do{

printf("\n1. ADD\n2. DELETE\n3. LIST\n4. EXIT");
scanf("%d",&c);

switch(c)
{
case 1:
printf("Enter Name\n");
scanf("%s%",st.ch);
printf("Enter Tail Number\n");
scanf("%s",st.number);
printf("Enter Price\n");
scanf("%f",&st.price);
printf("Enter Total Seats\n");
scanf("%d",&st.seats);
printf("Enter First Class Seats\n");
scanf("%d",&st.first);
st.other=st.seats-st.first;
a=adddata(&st);
if(a=0)
printf("ERROR ENTRY NOT ADDED");
else
printf("ENRTY ADDED SUCESSFULLY");
free(&st);
break;
case 2: a_delete();
break;
case 3:
display();
break;
case 4: exit(1);
}
}while(1);
}

definefunction.c

#include<stdio.h>
#include"inventory.h"
#include<fcntl.h>
#include<unistd.h>
#include<stdlib.h>
#include<errno.h>
#include<string.h>
#include<sys/stat.h>

int adddata(st_t *x)
{
int fd=0,fdw;
char path[]="records";
fd=open(path,O_CREAT|O_RDWR,0644);
if(fd<0)printf("error");
close(fd);
fd=open(path,O_CREAT|O_APPEND|O_RDWR);
if(fd<0)perror("open");
fdw=write(fd,x,sizeof(st_t));
if(fdw<=0)
perror("write");
close(fd);
return 1;
}

void display()
{
st_t z;
struct stat tt;
int n=0,i,fd;
char path[]="records";
stat(path,&tt);
i=tt.st_size;
fd=open(path,O_CREAT|O_RDONLY,0644);
if(fd<0)
printf("error");
printf(" Name\t Number\tPrice\tTotal Seats\tFirst Class\tother\n")
;
while(i>=84)
{
lseek(fd,n,SEEK_SET);
read(fd,&z,sizeof(st_t));
printf("%s\t\t%s\t\t%f\t%d\t%d\t\t%d\n",z.ch,z.number,z.price,z.seats,
z.first,z.other);
n=n+85;
i=i-84;
}
close(fd);

}

void a_delete()
{
int n=0,i,fd,fdt,s;
char path[]="records";
st_t tmp1,tmp2,buffer;
struct stat tt1,tt2;
printf("\nenter name to be deleted");
scanf("\n%s",tmp1.ch);
printf("%s",tmp1.ch);
stat(path,&tt1);
i=tt1.st_size;
fdt=open("temp",O_CREAT|O_RDWR,0644);
close(fdt);
fd = open(path,O_CREAT|O_RDWR,0644);
while(i>=84)
{
lseek(fd,n,SEEK_SET);
read(fd,&tmp2,sizeof(st_t));
if((s=strcmp(tmp1.flight,tmp2.flight)!=0))
{
fdt=open("temp",O_CREAT|O_RDWR|O_APPEND);
write(fdt,&tmp2,sizeof(st_t));
close(fdt);
}
n=n+85;
i=i-84;
}
close(fd);
stat("temp",&tt2);
i=tt2.st_size;
n=0;
fd=open(path,O_CREAT|O_RDWR|O_TRUNC,0644
);
close(fd);
fdt=open("temp",O_CREAT|O_RDWR,0644);
while(i>=84)
{
lseek(fdt,n,SEEK_SET);
fd=open(path,O_CREAT|O_RDWR|O_APPEND);
read(fdt,&buffer,sizeof(st_t));
write(fd,&buffer,sizeof(st_t));
close(fd);
n=n+85;
i=i-84;
}
close(fdt);
remove("temp");
}

Pascal Bourguignon

2005-09-27, 5:55 pm

"friend_05" <hirenshah.05@gmail.com> writes:

> here is my actual code.
> only my first data is written properly in file. after first data other
> data are written randomly in file. i thinl there is problem of buffer
> overflow but i am not able solve the problem.

Why don't you provide a makefile if you have several files?
What about :

all: inv
inv:definefunction.o mainprogram.o
gcc -o $@ $<

> this code has no errors

Not really:

-*- mode: compilation; default-directory: "/tmp/inv/" -*-
make -k
cc -c -o definefunction.o definefunction.c
definefunction.c: In function `a_delete':
definefunction.c:73: error: structure has no member named `flight'
definefunction.c:73: error: structure has no member named `flight'
make: *** [definefunction.o] Error 1
cc -c -o inventory.o inventory.c
make: Target `all' not remade because of errors.

Compilation exited abnormally with code 2 at Tue Sep 27 20:26:54

--
__Pascal Bourguignon__ http://www.informatimago.com/
The mighty hunter
Returns with gifts of plump birds,
Your foot just squashed one.

John Gordon

2005-09-27, 5:55 pm

In <1127844821.353661.74330@o13g2000cwo.googlegroups.com> "friend_05" <hirenshah.05@gmail.com> writes:

> here is my actual code.

> switch(c)
> {
> case 1:
> printf("Enter Name\n");
> scanf("%s%",st.ch);

What is that extra % doing there?

> st.other=st.seats-st.first;
> a=adddata(&st);
> if(a=0)

You probably wanted to use == here instead of =.

> printf("ERROR ENTRY NOT ADDED");
> else
> printf("ENRTY ADDED SUCESSFULLY");
> free(&st);

This is VERY VERY BAD. st did not come from malloc(), so you
shoud not free() it.

> void display()
> {
> st_t z;
> struct stat tt;
> int n=0,i,fd;
> char path[]="records";
> stat(path,&tt);
> i=tt.st_size;
> fd=open(path,O_CREAT|O_RDONLY,0644);
> if(fd<0)
> printf("error");

If fd<0, don't you want to exit the function?

> printf(" Name\t Number\tPrice\tTotal Seats\tFirst Class\tother\n")
> ;
> while(i>=84)
> {
> lseek(fd,n,SEEK_SET);
> read(fd,&z,sizeof(st_t));
> printf("%s\t\t%s\t\t%f\t%d\t%d\t\t%d\n",z.ch,z.number,z.price,z.seats,
> z.first,z.other);
> n=n+85;
> i=i-84;
> }
> close(fd);

This function is needlessly complicated. Why are you counting bytes?
Just read one structure at a time, and stop after you're read them all.
Like this:

void display()
{
st_t z;
char *path = "records";
int fd;
int nbytes;

fd = open(path,O_CREAT|O_RDONLY,0644);

/* this loop will execute as long as there is something to read */
while(read(fd,&z,sizeof(st_t)) > 0)
{
printf("%s\t\t%s\t\t%f\t%d\t%d\t\t%d\n",z.ch,z.number,z.price,z.seats, z.first,z.other);
}

close(fd);
}

--
John Gordon "It's certainly uncontaminated by cheese."
gordon@panix.com

Floyd L. Davidson

2005-09-27, 5:55 pm

"friend_05" <hirenshah.05@gmail.com> wrote:
>here is my actual code.
....

> case 1:
> printf("Enter Name\n");
> scanf("%s%",st.ch);

That is not doing what you want done. If you insist on using
scanf() for keyboard input, read the man page and learn about
what happens to white space in the input.

....
> printf("ENRTY ADDED SUCESSFULLY");
> free(&st);

You haven't malloc'ed space for st, so don't call free().

....
> fd=open(path,O_CREAT|O_RDWR,0644);
> if(fd<0)printf("error");

If there is an error, what point is there in continuing just
as if there isn't? Either fix the error, or exit.

But is the purpose this code? It isn't needed!

> close(fd);
> fd=open(path,O_CREAT|O_APPEND|O_RDWR);

Add the third argument to open(), and delete the first call to open().

> if(fd<0)perror("open");

So... *do* something about it! :-)

> fdw=write(fd,x,sizeof(st_t));
> if(fdw<=0)
> perror("write");

What's the point of just continuing?

> close(fd);
> return 1;

Why have this function return a value, and then just
always return the same value? It might as well return void.

>}
>
>void display()
>{
> st_t z;
> struct stat tt;
> int n=0,i,fd;
> char path[]="records";

This should probably be a global char array, just
to make sure all of these functions are working with
the same file.

> stat(path,&tt);
> i=tt.st_size;
> fd=open(path,O_CREAT|O_RDONLY,0644);

What is the purpose in using O_CREAT with a read only file????

> if(fd<0)
> printf("error");

If there is an error, the rest of this function is going to have
run time problems...

> printf(" Name\t Number\tPrice\tTotal Seats\tFirst Class\tother\n")
>;
> while(i>=84)

Eh? Don't do this!

Read the file from beginning to the end. When you try to read more
data, if you get nothing... it's *done*! Just use the return value
from read().

I made one pass through that code, and I'm sure to have missed
several other equally bad problems. Virtually all of the above
problems are repeated too. But your coding style is all but
impossible to read, so I don't see how you can see what is wrong
with it... and I'm not going to get eye strain from looking at
it in detail!

--
Floyd L. Davidson <http://www.apaflo.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) floyd@apaflo.com

Floyd L. Davidson

2005-09-27, 5:55 pm

Pascal Bourguignon <spam@mouse-potato.com> wrote:
>"friend_05" <hirenshah.05@gmail.com> writes:
>
>
>Why don't you provide a makefile if you have several files?
>What about :
>
>all: inv
>inv:definefunction.o mainprogram.o
> gcc -o $@ $<

NO!

He needs to turn on a whole raft of options. Here's the command
I use by default for testing short snippets of code:

gcc -ggdb -O2 -std=c99 -pedantic -Wall -W -Wcast-align \
-Wcast-qual -Wmissing-prototypes -Wshadow -Wnested-externs \
-Wstrict-prototypes -Waggregate-return -c foo.c

The Makefile looks something like this:

#
# Turn on just about everything reasonably possible
#
GCC_WFLAGS = -Wcast-align -Wcast-qual -Wmissing-prototypes \
-Wshadow -Wnested-externs -Wstrict-prototypes \
-Waggregate-return # -Wtraditional #-Wpointer-arith

#
# Comment out as needed...
#
GCC_OPTIM = -O2
GCC_SYMTAB = -ggdb # -I "${QTDIR}/include"
GCC_ANSI = -std=c99
GCC_PEDANT = -pedantic

#
GCC_DEBUG = ${GCC_SYMTAB} ${GCC_OPTIM} ${GCC_ANSI} ${GCC_PEDANT} -Wall -W
CC = gcc
LD = ${CC} #-L /usr/X11R6/lib

LIBS = #-lncurses #-lutil -lXt #-lncurses -lm #-lreadline
OPTS = ${GCC_DEBUG} ${GCC_WFLAGS} ${GCC_FFLAGS}

SRCS = foo.c
OBJS = $(SRCS:.c=.o)
TARGETS = foo

all: $(TARGETS)

.c.o:
$(CC) $(OPTS) -c $<

foo: ${OBJS}
${LD} ${OBJS} $(LIBS) $(LDOPTS) -o $@

And is impossible to read... ;-)
[vbcol=seagreen]
>Not really:

....
>definefunction.c:73: error: structure has no member named `flight'
....

--
Floyd L. Davidson <http://www.apaflo.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) floyd@apaflo.com

Bit Twister

2005-09-28, 2:50 am

On 27 Sep 2005 11:13:41 -0700, friend_05 wrote:
> here is my actual code.
> only my first data is written properly in file. after first data other
> data are written randomly in file. i thinl there is problem of buffer
> overflow but i am not able solve the problem.
> this code has no errors

You have several errors.

Run your code through lint or lclint