Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem but when i run it i face some other
ploblems which i can not correct.I would appreciated if someone take a
look at my programm so as to help me.I tried many times to find out
where my mistakes are but i didn't manage something.
I have made some comments due to the programm so as to make it more
easy to as.
Any recommendations about my writing style in the programm are
acceptable.
Please help me!!!
Programm
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
/* Functions*/
int menu(void);
struct sells *insert(void);
struct sells *search(void);
int show(struct sells *list);
int exit(void);
/*Struct*/
struct sells{
char *tittle;
char *singer;
int price;
int count;
struct sells *next;
};
struct sells *list;
struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
}
/*Function main*/
int main(void)
{
menu();
return 0;
}
/*Function menu.In this function the user has the opportunity to choose
what he/she want to do*/
int menu(void)
{
int i;
printf("---------MENU---------\n");
printf("Press \"1\" to insert data\n");
printf("Press \"2\" to search data\n");
printf("Press \"3\" to show data\n");
printf("Press \"4\" to exit from programm\n");
putchar('\n\n');
printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
{
printf("You made wrong choice.\nPlease make a new
choice\n");
scanf("%d",&i);
}
switch(i)
{
case 1:
{
insert();
break;
}
case 2:
{
search();
break;
}
case 3:
{
show(list);
break;
}
case 4:
{
exit();
break;
}
}
return i;
}
/*In this function the user insert the data.*/
struct sells *insert(void)
{
char titlos[15];
char tragoudisths[15];
int timi;
struct sells *n;
float m_out;
int i;
m_out=0;
printf("%f\n",&m_out);
n=alloc();
printf("Give the title of the CD\n");
scanf("%s",&titlos);
getchar();
printf("Give the name of the singer\n");
scanf("%s",&tragoudisths);
getchar();
printf("Give the price of the CD\n");
scanf("%d",&timi);
getchar();
while((timi<0)||(timi>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&timi);
getchar();
}
getchar();
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
if(list==NULL)
{
list=alloc();
list->tittle=strdup(n->tittle);
list->singer=strdup(n->singer);
list->price=n->price;
list->next=NULL;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))==0)
{
list->count++;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))<0)
{
n=alloc();
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
n->count=1;
n->next=list;
list=n;
printf("%s\t\t%s\t\t%d\t\t\n",list->tittle,list->singer,list->price);
}
else
{
list->next=insert();
return list;
}
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return n;
}
/*In this function the user search for an element.Also he can modify
it.
If someone can i would like to tell me what i have to do so as to
delete an element.*/
struct sells *search(void)
{
char titlos[15];
int i,k,out;
char choice;
printf("Give the tittle of the CD that you want to find\n");
gets(titlos);
getchar();
if(list==NULL||(out=strcmp(list->tittle,titlos))>0)
{
printf("The tittle that you want doesn't exist.\n");
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return list;
}
else if(out==0)
{
printf("Tittle: \t%s\n",list->tittle);
printf("Singer: \t%s\n",list->singer);
printf("Price: \t%d\n",list->price);
printf("Do you want to make any change;\n");
printf("If Yes press \"1\" else \"0\" to return to
menu\n");
printf("Make your choice");
scanf("%d",&k);
switch(k)
{
case 1:
{
printf("Do you want ot change the tittle;\nPress Y(YES)
or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new tittle:\n");
gets(list->tittle);
getchar();
}
printf("Do you want ot change the name of the
singer;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new name:\n");
gets(list->singer);
getchar();
}
printf("Do you want ot change the price of the
CD;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new price:\n");
scanf("%d",&list->price);
while((list->price<0)&&(list->price>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&list->price);
}
putchar('\n');
}
break;
}
case 0:
{
menu();
break;
}
}
}
return list;
}
/*Function which shows to the user all the element that has already
insert.
In this functio there is a problem.When i insert more the 3 elements,
these elements doesn't appear to me.I think that the problem is in my
insert
but i am not quite sure.*/
int show(struct sells *list)
{
int i,k;
k=1;
printf("\t\t-------LIST--------\n");
printf("Num\t\tTittle\t\tSinger\t\tPrice\n");
while(list!=NULL)
{
printf("%d\t%s\t\t%s\t\t%d\t\t\n",k,list->tittle,list->singer,list->price);
k++;
list=list->next;
}
printf("What do you want to do\n");
printf("Press \"1\" to retutn to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return 0;
}
/*Exit from the programm*/
int exit(void)
{
int i;
printf("Do you really want to exit from the programm;\n");
printf("If Yes press \"1\" else \"0\"\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
break;
}
return 0;
}
}
83  7688  de*****@hotmail.com said:
Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem
I beg to differ. See below. Fix this lot, starting with the first, and then
get back to us.
(Caution: in the following diagnostic report, line numbers may differ
slightly to those in your source.)
gcc -W -Wall -ansi -pedantic -Wformat-nonliteral -Wcast-align
-Wpointer-arith -Wbad-function-cast -Wmissing-prototypes
-Wstrict-prototypes -Wmissing-declarations -Winline -Wundef
-Wnested-externs -Wcast-qual -Wshadow -Wconversion -Wwrite-strings
-Wno-conversion -ffloat-store -O2 -g -pg -c -o foo.o foo.c
foo.c:10: conflicting types for `exit'
/usr/include/stdlib.h:577: previous declaration of `exit'
foo.c:26: warning: no previous prototype for `alloc'
foo.c: In function `menu':
foo.c:52: warning: multi-character character constant
foo.c:59: warning: unknown escape sequence `\ '
foo.c: In function `insert':
foo.c:102: warning: double format, pointer arg (arg 2)
foo.c:105: warning: char format, different type arg (arg 2)
foo.c:108: warning: char format, different type arg (arg 2)
foo.c:115: warning: unknown escape sequence `\ '
foo.c:116: warning: unknown conversion type character ` ' in format
foo.c:116: warning: too many arguments for format
foo.c:120: warning: implicit declaration of function `strdup'
foo.c:120: warning: assignment makes pointer from integer without a cast
foo.c:121: warning: assignment makes pointer from integer without a cast
foo.c:126: warning: assignment makes pointer from integer without a cast
foo.c:127: warning: assignment makes pointer from integer without a cast
foo.c:130: warning: double format, pointer arg (arg 2)
foo.c:135: warning: double format, pointer arg (arg 2)
foo.c:140: warning: assignment makes pointer from integer without a cast
foo.c:141: warning: assignment makes pointer from integer without a cast
foo.c: In function `search':
foo.c:214: warning: unknown escape sequence `\ '
foo.c:222: warning: unknown escape sequence `\ '
foo.c:223: warning: unknown conversion type character ` ' in format
foo.c:223: warning: too many arguments for format
foo.c:232: warning: unknown escape sequence `\ '
foo.c:242: warning: unknown escape sequence `\ '
foo.c:252: warning: unknown escape sequence `\ '
foo.c:253: warning: unknown conversion type character ` ' in format
foo.c:253: warning: too many arguments for format
foo.c: At top level:
foo.c:276: warning: declaration of `list' shadows global declaration
foo.c: In function `show':
foo.c:277: warning: declaration of `list' shadows global declaration
foo.c: In function `exit':
foo.c:335: warning: control reaches end of non-void function
make: *** [foo.o] Error 1
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously) de*****@hotmail.com napsal(a):
Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem but when i run it i face some other
ploblems which i can not correct.I would appreciated if someone take a
look at my programm so as to help me.I tried many times to find out
where my mistakes are but i didn't manage something.
I have made some comments due to the programm so as to make it more
easy to as.
Any recommendations about my writing style in the programm are
acceptable.
Coding style? yes, try to use indent in way such this one:
indent -npro -kr -i8 -ts8 -sob -l80 -ss -ncs
(un*x utility for indentation)
Please help me!!!
Programm
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
/* Functions*/
int menu(void);
struct sells *insert(void);
struct sells *search(void);
int show(struct sells *list);
int exit(void);
/*Struct*/
struct sells{
char *tittle;
char *singer;
int price;
int count;
struct sells *next;
};
struct sells *list;
struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
you don't need to cast.
}
/*Function main*/
int main(void)
{
menu();
return 0;
}
/*Function menu.In this function the user has the opportunity to choose
what he/she want to do*/
int menu(void)
{
int i;
printf("---------MENU---------\n");
printf("Press \"1\" to insert data\n");
printf("Press \"2\" to search data\n");
printf("Press \"3\" to show data\n");
printf("Press \"4\" to exit from programm\n");
You may use puts for these.
putchar('\n\n');
printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
{
printf("You made wrong choice.\nPlease make a new
choice\n");
scanf("%d",&i);
}
switch(i)
{
case 1:
{
insert();
break;
}
case 2:
{
search();
break;
}
case 3:
{
show(list);
break;
}
case 4:
{
exit();
break;
}
}
return i;
}
/*In this function the user insert the data.*/
struct sells *insert(void)
{
char titlos[15];
char tragoudisths[15];
int timi;
struct sells *n;
float m_out;
int i;
m_out=0;
you can do float m_out = 0;
printf("%f\n",&m_out);
n=alloc();
check for NULL.
>
printf("Give the title of the CD\n");
scanf("%s",&titlos);
This is bug-prone. If I enter char 15, it will crash.
Another thing, turn on warnings, it would tell you, it's bad.
titlos is pointer yet, so scanf("%s", titlos); is correct,
getchar();
printf("Give the name of the singer\n");
scanf("%s",&tragoudisths);
....
getchar();
printf("Give the price of the CD\n");
scanf("%d",&timi);
getchar();
while((timi<0)||(timi>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&timi);
getchar();
}
getchar();
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
NULLs handling
n->price=timi;
if(list==NULL)
{
list=alloc();
again
list->tittle=strdup(n->tittle);
list->singer=strdup(n->singer);
list->price=n->price;
list->next=NULL;
printf("%f",&m_out);
why pointer?
}
else if((m_out=(strcmp(titlos,n->tittle)))==0)
{
list->count++;
printf("%f",&m_out);
....
}
else if((m_out=(strcmp(titlos,n->tittle)))<0)
{
n=alloc();
....
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
n->count=1;
n->next=list;
list=n;
printf("%s\t\t%s\t\t%d\t\t\n",list->tittle,list->singer,list->price);
}
else
{
list->next=insert();
return list;
}
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return n;
}
/*In this function the user search for an element.Also he can modify
it.
If someone can i would like to tell me what i have to do so as to
delete an element.*/
struct sells *search(void)
{
char titlos[15];
int i,k,out;
char choice;
printf("Give the tittle of the CD that you want to find\n");
gets(titlos);
getchar();
if(list==NULL||(out=strcmp(list->tittle,titlos))>0)
{
printf("The tittle that you want doesn't exist.\n");
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return list;
}
else if(out==0)
{
printf("Tittle: \t%s\n",list->tittle);
printf("Singer: \t%s\n",list->singer);
printf("Price: \t%d\n",list->price);
printf("Do you want to make any change;\n");
printf("If Yes press \"1\" else \"0\" to return to
menu\n");
printf("Make your choice");
scanf("%d",&k);
switch(k)
{
case 1:
{
printf("Do you want ot change the tittle;\nPress Y(YES)
or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new tittle:\n");
gets(list->tittle);
getchar();
}
printf("Do you want ot change the name of the
singer;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new name:\n");
gets(list->singer);
getchar();
}
printf("Do you want ot change the price of the
CD;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new price:\n");
scanf("%d",&list->price);
while((list->price<0)&&(list->price>100))
{
unneeded { }
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&list->price);
}
putchar('\n');
}
break;
}
case 0:
{
menu();
break;
}
}
}
return list;
}
/*Function which shows to the user all the element that has already
insert.
In this functio there is a problem.When i insert more the 3 elements,
these elements doesn't appear to me.I think that the problem is in my
insert
but i am not quite sure.*/
int show(struct sells *list)
{
int i,k;
k=1;
printf("\t\t-------LIST--------\n");
printf("Num\t\tTittle\t\tSinger\t\tPrice\n");
while(list!=NULL)
{
printf("%d\t%s\t\t%s\t\t%d\t\t\n",k,list->tittle,list->singer,list->price);
k++;
list=list->next;
}
printf("What do you want to do\n");
printf("Press \"1\" to retutn to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return 0;
}
/*Exit from the programm*/
int exit(void)
{
int i;
printf("Do you really want to exit from the programm;\n");
printf("If Yes press \"1\" else \"0\"\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
break;
}
Why you call menu again, when you want to exit? Call exit(0);, but cleanup before.
return 0;
}
}
--
js
On Fri, 07 Jul 2006 08:38:54 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
>de*****@hotmail.com said:
>Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem
I beg to differ. See below. Fix this lot, starting with the first, and then
get back to us.
(Caution: in the following diagnostic report, line numbers may differ
slightly to those in your source.)
gcc -W -Wall -ansi -pedantic -Wformat-nonliteral -Wcast-align
-Wpointer-arith -Wbad-function-cast -Wmissing-prototypes
-Wstrict-prototypes -Wmissing-declarations -Winline -Wundef
-Wnested-externs -Wcast-qual -Wshadow -Wconversion -Wwrite-strings
-Wno-conversion -ffloat-store -O2 -g -pg -c -o foo.o foo.c
And then top it off with a run through PC-lint from http://www.gimpel.com/ (warnings like 525 are supressed, and line
numbers may differ slightly to those in your source):
PC-lint for C/C++ (NT) Vers. 8.00u, Copyright Gimpel Software
1985-2006
--- Module: stdc.c (C)
_
int exit(void);
stdc.c(10) : Error 18: Symbol 'exit(int)' redeclared (void/nonvoid,
arg. count) conflicts with line 261, file stdlib.h
stdlib.h(261) : Info 830: Location cited in prior message
_
int exit(void);
stdc.c(10) : Warning 532: Return mode of function 'exit(int)'
inconsistent with line 261, file stdlib.h
stdlib.h(261) : Info 830: Location cited in prior message
_
menu();
stdc.c(33) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 6)
stdc.c(6) : Info 830: Location cited in prior message
_
#... = (char)(('\n\n'))) : _flsbuf((('\n\n')),((&_iob[1])))) /*lint
-restore *
#... ('\n\n'),stdout)
putchar('\n\n');
stdc.c(51) : Info 742: Multiple character constant
_
#... _flsbuf((('\n\n')),((&_iob[1])))) /*lint -restore */
#... ('\n\n'),stdout)
putchar('\n\n');
stdc.c(51) : Info 742: Multiple character constant
_
insert();
stdc.c(66) : Warning 534: Ignoring return value of function
'insert(void)' (compare with line 7)
stdc.c(7) : Info 830: Location cited in prior message
_
search();
stdc.c(71) : Warning 534: Ignoring return value of function
'search(void)' (compare with line 8)
stdc.c(8) : Info 830: Location cited in prior message
_
show(list);
stdc.c(76) : Warning 534: Ignoring return value of function
'show(struct sells *)' (compare with line 9)
stdc.c(9) : Info 830: Location cited in prior message
_
exit();
stdc.c(81) : Error 118: Too few arguments (0) for prototype
'exit(int)'
_
}
stdc.c(84) : Info 744: switch statement has no default
_
printf("%f\n",&m_out);
stdc.c(100) : Warning 559: Size of argument no. 2 inconsistent with
format
_
scanf("%s",&titlos);
stdc.c(104) : Warning 545: Suspicious use of &
stdc.c(104) : Warning 561: (arg. no. 2) indirect object inconsistent
with format
_
getchar();
stdc.c(105) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
scanf("%s",&tragoudisths);
stdc.c(109) : Warning 545: Suspicious use of &
stdc.c(109) : Warning 561: (arg. no. 2) indirect object inconsistent
with format
_
getchar();
stdc.c(110) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
getchar();
stdc.c(114) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
getchar();
stdc.c(120) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
getchar();
stdc.c(122) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
printf("%f",&m_out);
stdc.c(135) : Warning 559: Size of argument no. 2 inconsistent with
format
_
printf("%f",&m_out);
stdc.c(140) : Warning 559: Size of argument no. 2 inconsistent with
format
_
printf("Press \"1\" to return to menu or \"2\" to exit\n");
stdc.c(159) : Warning 539: Did not expect positive indentation from
line 154
stdc.c(154) : Info 830: Location cited in prior message
_
printf("Press \"1\" to return to menu or \"2\" to exit\n");
stdc.c(159) : Warning 539: Did not expect positive indentation from
line 142
stdc.c(142) : Info 830: Location cited in prior message
_
printf("Press \"1\" to return to menu or \"2\" to exit\n");
stdc.c(159) : Warning 539: Did not expect positive indentation from
line 137
stdc.c(137) : Info 830: Location cited in prior message
_
menu();
stdc.c(167) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 42)
stdc.c(42) : Info 830: Location cited in prior message
_
exit();
stdc.c(172) : Error 118: Too few arguments (0) for prototype
'exit(int)'
_
}
stdc.c(175) : Info 744: switch statement has no default
_
gets(titlos);
stdc.c(191) : Warning 534: Ignoring return value of function
'gets(char *)' (compare with line 324, file stdio.h)
stdio.h(324) : Info 830: Location cited in prior message
_
gets(titlos);
stdc.c(191) : Warning 421: Caution -- function 'gets(char *)' is
considered dangerous
_
getchar();
stdc.c(192) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
menu();
stdc.c(204) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 42)
stdc.c(42) : Info 830: Location cited in prior message
_
exit();
stdc.c(209) : Error 118: Too few arguments (0) for prototype
'exit(int)'
_
}
stdc.c(212) : Info 744: switch statement has no default
_
getchar();
stdc.c(231) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
gets(list->tittle);
stdc.c(235) : Warning 534: Ignoring return value of function
'gets(char *)' (compare with line 324, file stdio.h)
stdio.h(324) : Info 830: Location cited in prior message
_
gets(list->tittle);
stdc.c(235) : Warning 421: Caution -- function 'gets(char *)' is
considered dangerous
_
getchar();
stdc.c(236) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
getchar();
stdc.c(240) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
gets(list->singer);
stdc.c(244) : Warning 534: Ignoring return value of function
'gets(char *)' (compare with line 324, file stdio.h)
stdio.h(324) : Info 830: Location cited in prior message
_
gets(list->singer);
stdc.c(244) : Warning 421: Caution -- function 'gets(char *)' is
considered dangerous
_
getchar();
stdc.c(245) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
getchar();
stdc.c(249) : Warning 534: Ignoring return value of function
'_filbuf(struct _iobuf *)' (compare with line 275, file stdio.h)
stdio.h(275) : Info 830: Location cited in prior message
_
scanf("%d",&list->price);
stdc.c(257) : Info 725: Expected positive indentation from line 254
stdc.c(254) : Info 830: Location cited in prior message
_
menu();
stdc.c(265) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 42)
stdc.c(42) : Info 830: Location cited in prior message
_
}
stdc.c(268) : Info 744: switch statement has no default
_
{
stdc.c(281) : Warning 578: Declaration of symbol 'list' hides symbol
'list' (line 21)
stdc.c(21) : Info 830: Location cited in prior message
_
menu();
stdc.c(303) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 42)
stdc.c(42) : Info 830: Location cited in prior message
_
exit();
stdc.c(308) : Error 118: Too few arguments (0) for prototype
'exit(int)'
_
}
stdc.c(311) : Info 744: switch statement has no default
_
{
stdc.c(319) : Error 18: Symbol 'exit(int)' redeclared (void/nonvoid,
arg. count) conflicts with line 261, file stdlib.h
stdlib.h(261) : Info 830: Location cited in prior message
_
{
stdc.c(319) : Warning 515: Symbol 'exit(void)' has arg. count conflict
(0 vs. 1) with line 261, file stdlib.h
stdlib.h(261) : Info 830: Location cited in prior message
_
{
stdc.c(319) : Warning 532: Return mode of function 'exit(void)'
inconsistent with line 261, file stdlib.h
stdlib.h(261) : Info 830: Location cited in prior message
_
menu();
stdc.c(331) : Warning 534: Ignoring return value of function
'menu(void)' (compare with line 42)
stdc.c(42) : Info 830: Location cited in prior message
_
return 0;
stdc.c(338) : Warning 527: Unreachable code at token 'return'
_
}
stdc.c(339) : Info 744: switch statement has no default
_
}
stdc.c(340) : Warning 533: function 'exit(void)' should return a value
(see line 318)
stdc.c(318) : Info 830: Location cited in prior message
--- Global Wrap-up
Note 900: Successful completion, 91 messages produced
Tool returned code: 0
--
jay
jaysome said:
<snip>
And then top it off with a run through PC-lint from
http://www.gimpel.com/ (warnings like 525 are supressed, and line
numbers may differ slightly to those in your source):
Oooh, unkind. :-)
Some good news for the OP: my error list was somewhat inflated by my
cack-handed attempts at linewrap repair. Here is the revised list, which is
a fair bit shorter:
gcc -W -Wall -ansi -pedantic -Wformat-nonliteral -Wcast-align
-Wpointer-arith -Wbad-function-cast -Wmissing-prototypes
-Wstrict-prototypes -Wmissing-declarations -Winline -Wundef
-Wnested-externs -Wcast-qual -Wshadow -Wconversion -Wwrite-strings
-Wno-conversion -ffloat-store -O2 -g -pg -c -o foo.o foo.c
foo.c:10: conflicting types for `exit'
/usr/include/stdlib.h:577: previous declaration of `exit'
foo.c:26: warning: no previous prototype for `alloc'
foo.c: In function `menu':
foo.c:52: warning: multi-character character constant
foo.c: In function `insert':
foo.c:102: warning: double format, pointer arg (arg 2)
foo.c:105: warning: char format, different type arg (arg 2)
foo.c:108: warning: char format, different type arg (arg 2)
foo.c:120: warning: implicit declaration of function `strdup'
foo.c:120: warning: assignment makes pointer from integer without a cast
foo.c:121: warning: assignment makes pointer from integer without a cast
foo.c:126: warning: assignment makes pointer from integer without a cast
foo.c:127: warning: assignment makes pointer from integer without a cast
foo.c:130: warning: double format, pointer arg (arg 2)
foo.c:135: warning: double format, pointer arg (arg 2)
foo.c:140: warning: assignment makes pointer from integer without a cast
foo.c:141: warning: assignment makes pointer from integer without a cast
foo.c: At top level:
foo.c:276: warning: declaration of `list' shadows global declaration
foo.c: In function `show':
foo.c:277: warning: declaration of `list' shadows global declaration
foo.c: In function `exit':
foo.c:335: warning: control reaches end of non-void function
make: *** [foo.o] Error 1
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
On Fri, 07 Jul 2006 09:10:14 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
>jaysome said:
<snip>
>And then top it off with a run through PC-lint from
http://www.gimpel.com/ (warnings like 525 are supressed, and line
numbers may differ slightly to those in your source):
Oooh, unkind. :-)
One of the warnings, 421, flagged gets() as being dangerous. The
PC-lint text documentation (MSG.TXT), which you can download from
Gimpel's web site, says this about warning 421:
421 Caution -- function 'Symbol' is considered dangerous -- This
message is issued (by default) for the built-in function gets.
This function is considered dangerous because there is no
mechanism to ensure that the buffer provided as first argument
will not overflow. A well known computer virus (technically a
worm) was created based on this defect. Through the -function
option, the user may designate other functions as dangerous.
What other Standard C functions would be good candidates for the
"-function" option?
Best regards
--
jay
jaysome said:
On Fri, 07 Jul 2006 09:10:14 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
>>jaysome said:
<snip>
>>And then top it off with a run through PC-lint from
http://www.gimpel.com/ (warnings like 525 are supressed, and line
numbers may differ slightly to those in your source):
Oooh, unkind. :-)
One of the warnings, 421, flagged gets() as being dangerous. The
PC-lint text documentation (MSG.TXT), which you can download from
Gimpel's web site, says this about warning 421:
<snip>
>
What other Standard C functions would be good candidates for the
"-function" option?
Just gets(). But I'd flag a few others as being "experts-only" functions,
which you should not use in "real" code unless you know /exactly/ what
you're doing:
From <stdio.h>
freopen, fflush, setvbuf, setbuf, sprintf, vsprintf, fscanf,
scanf, sscanf, feof
From <string.h>
strncpy, strncat, strtok, memcpy, memset
From <stdlib.h>
atof, atoi, atol, calloc, malloc, realloc, free, atexit,
system
From <stdarg.h>
va_start, va_arg, va_end
From <setjmp.h>
setjmp, longjmp
From <signal.h>
signal
From <time.h>
clock
As a matter of fact, I was tempted to list every single function in the
standard library! But the ones I've listed do, I think, represent the
principal "gotcha" functions.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
jaysome <ja*****@spamcop.netwrote:
One of the warnings, 421, flagged gets() as being dangerous. The
PC-lint text documentation (MSG.TXT), which you can download from
Gimpel's web site, says this about warning 421:
421 Caution -- function 'Symbol' is considered dangerous -- This
message is issued (by default) for the built-in function gets.
This function is considered dangerous because there is no
mechanism to ensure that the buffer provided as first argument
will not overflow. A well known computer virus (technically a
worm) was created based on this defect. Through the -function
option, the user may designate other functions as dangerous.
What other Standard C functions would be good candidates for the
"-function" option?
None, really. gets() is the only one that can _not_ be used safely.
Functions like scanf() are easy to use unsafely, but can be made safe in
a simple, though WOMBATty, way, by constructing the format string at run
time, using something like (warning: not tested!)
sprintf(format_string, "%%%ds", current_size_of_buffer);
scanf(format_string, destination_string);
Richard
Richard Bos said:
<snip>
Functions like scanf() are easy to use unsafely, but can be made safe in
a simple, though WOMBATty, way, by constructing the format string at run
time, using something like (warning: not tested!)
sprintf(format_string, "%%%ds", current_size_of_buffer);
scanf(format_string, destination_string);
You'll want to add a check on the return value of scanf.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
<de*****@hotmail.comwrote:
<just some random observations here and there>
Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem but when i run it i face some other
ploblems which i can not correct.I would appreciated if someone take a
look at my programm so as to help me.I tried many times to find out
where my mistakes are but i didn't manage something.
I have made some comments due to the programm so as to make it more
easy to as.
Any recommendations about my writing style in the programm are
acceptable.
Please help me!!!
Programm
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
/* Functions*/
int menu(void);
struct sells *insert(void);
struct sells *search(void);
int show(struct sells *list);
int exit(void);
exit() is an existing fucntion in <stdlib.h It is not a good idea to reuse
names like this. Name your function exitx().
I wouldn't knowingly reuse names even if I *hadn't* included the particualar
include file.
/*Struct*/
struct sells{
char *tittle;
char *singer;
int price;
int count;
struct sells *next;
};
struct sells *list;
struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
}
/*Function main*/
int main(void)
{
menu();
return 0;
No data there. Your comments are detailed in nature, not "global". So I
can't tell what you are trying to do. I *guess* it is a linked list data
structure. But it is not owned by main, which would be typical. So I guess
it must be owned by menu. I'll look there.
}
/*Function menu.In this function the user has the opportunity to choose
what he/she want to do*/
int menu(void)
{
int i;
printf("---------MENU---------\n");
No, the only thing menu owns is a simple int. Who owns the list??? (If
there is a list.) Go to end.
printf("Press \"1\" to insert data\n");
printf("Press \"2\" to search data\n");
printf("Press \"3\" to show data\n");
printf("Press \"4\" to exit from programm\n");
putchar('\n\n');
printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
{
printf("You made wrong choice.\nPlease make a new
choice\n");
scanf("%d",&i);
}
switch(i)
{
case 1:
{
insert();
break;
}
case 2:
{
search();
break;
}
case 3:
{
show(list);
break;
}
case 4:
{
exit();
break;
}
}
return i;
}
/*In this function the user insert the data.*/
struct sells *insert(void)
{
char titlos[15];
char tragoudisths[15];
int timi;
struct sells *n;
float m_out;
int i;
m_out=0;
printf("%f\n",&m_out);
n=alloc();
printf("Give the title of the CD\n");
scanf("%s",&titlos);
getchar();
printf("Give the name of the singer\n");
scanf("%s",&tragoudisths);
getchar();
printf("Give the price of the CD\n");
scanf("%d",&timi);
getchar();
while((timi<0)||(timi>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&timi);
getchar();
}
getchar();
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
if(list==NULL)
{
list=alloc();
list->tittle=strdup(n->tittle);
list->singer=strdup(n->singer);
list->price=n->price;
list->next=NULL;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))==0)
{
list->count++;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))<0)
{
n=alloc();
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
n->count=1;
n->next=list;
list=n;
printf("%s\t\t%s\t\t%d\t\t\n",list->tittle,list->singer,list->price);
}
else
{
list->next=insert();
return list;
}
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return n;
}
/*In this function the user search for an element.Also he can modify
it.
If someone can i would like to tell me what i have to do so as to
delete an element.*/
struct sells *search(void)
{
char titlos[15];
int i,k,out;
char choice;
printf("Give the tittle of the CD that you want to find\n");
gets(titlos);
getchar();
if(list==NULL||(out=strcmp(list->tittle,titlos))>0)
{
printf("The tittle that you want doesn't exist.\n");
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return list;
}
else if(out==0)
{
printf("Tittle: \t%s\n",list->tittle);
printf("Singer: \t%s\n",list->singer);
printf("Price: \t%d\n",list->price);
printf("Do you want to make any change;\n");
printf("If Yes press \"1\" else \"0\" to return to
menu\n");
printf("Make your choice");
scanf("%d",&k);
switch(k)
{
case 1:
{
printf("Do you want ot change the tittle;\nPress Y(YES)
or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new tittle:\n");
gets(list->tittle);
getchar();
}
printf("Do you want ot change the name of the
singer;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new name:\n");
gets(list->singer);
getchar();
}
printf("Do you want ot change the price of the
CD;\nPress Y(YES) or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new price:\n");
scanf("%d",&list->price);
while((list->price<0)&&(list->price>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&list->price);
}
putchar('\n');
}
break;
}
case 0:
{
menu();
break;
}
}
}
return list;
}
/*Function which shows to the user all the element that has already
insert.
In this functio there is a problem.When i insert more the 3 elements,
these elements doesn't appear to me.I think that the problem is in my
insert
but i am not quite sure.*/
int show(struct sells *list)
{
int i,k;
k=1;
printf("\t\t-------LIST--------\n");
printf("Num\t\tTittle\t\tSinger\t\tPrice\n");
while(list!=NULL)
{
printf("%d\t%s\t\t%s\t\t%d\t\t\n",k,list->tittle,list->singer,list->price);
k++;
list=list->next;
}
printf("What do you want to do\n");
printf("Press \"1\" to retutn to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
return 0;
}
/*Exit from the programm*/
int exit(void)
{
int i;
printf("Do you really want to exit from the programm;\n");
Why do you assume the user is a moron? He just told you he wanted to exit.
printf("If Yes press \"1\" else \"0\"\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
break;
}
return 0;
}
}
I would start over. Don't do the menu, do it last, after the fundamentals
are working. Start with sell and show and get them working. Don't do all
the fussy details. For example, you can add reasonableness checks later.
Do you really need five fields in your data structure to test your ideas? I
doubt it. Programs should grow, Start small, test, add, test, add, test,
......
Richard Heathfield wrote:
jaysome said:
>On Fri, 07 Jul 2006 09:10:14 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
<snip>
>What other Standard C functions would be good candidates for the
"-function" option?
Just gets(). But I'd flag a few others as being "experts-only" functions,
which you should not use in "real" code unless you know /exactly/ what
you're doing:
<snip>
From <stdlib.h>
[...] calloc, malloc, realloc, free, [...]
Let's get this straight: you shouldn't dynamically allocate memory in C
unless you're an expert?
Well, actually, no arguments there -- but why not just say C is for experts
and leave it at that, then? For most applications, you need to be an expert
to write a program *without* dynamic memory allocation!
S.
Skarmander said:
Richard Heathfield wrote:
<snip>
>>
[...] calloc, malloc, realloc, free, [...]
Let's get this straight: you shouldn't dynamically allocate memory in C
unless you're an expert?
Absolutely. Possibly the biggest single issue I have with code posted here
is shoddy dynamic memory allocation code.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
On 2006-07-07, de*****@hotmail.com <de*****@hotmail.comwrote:
Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem but when i run it i face some other
ploblems which i can not correct.I would appreciated if someone take a
look at my programm so as to help me.I tried many times to find out
where my mistakes are but i didn't manage something.
I have made some comments due to the programm so as to make it more
easy to as.
Any recommendations about my writing style in the programm are
acceptable.
Please help me!!!
Actually, this is one of the most well-styled programs I've seen posted
here (by a novice) in a while. I recommend you use 2 or 4-space tabs
instead of 8.
Programm
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
/* Functions*/
int menu(void);
struct sells *insert(void);
struct sells *search(void);
int show(struct sells *list);
int exit(void);
/*Struct*/
struct sells{
char *tittle;
char *singer;
int price;
int count;
struct sells *next;
};
struct sells *list;
struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
No need for the cast. Use
return malloc (sizeof (struct sells));
(Actually, I question why you're using a function at all for this.)
}
/*Function main*/
int main(void)
{
menu();
return 0;
}
Even in small functions, formatting is essential.
>
/*Function menu.In this function the user has the opportunity to choose
what he/she want to do*/
int menu(void)
{
int i;
printf("---------MENU---------\n");
printf("Press \"1\" to insert data\n");
printf("Press \"2\" to search data\n");
printf("Press \"3\" to show data\n");
printf("Press \"4\" to exit from programm\n");
It would perhaps be more efficient (and easier to read) if you replaced
printf("...\n"); with puts("...");
Notice how I eliminated the \n that way. Also, you might want to replace
your \"escaped double quotes\" with ``manual double quotes'' (notice how
they bend in the right direction).
putchar('\n\n');
This isn't correct. You can put two characters into single quotes. You'll
need to do printf ("\n\n"), two puts("") or two putchar ('\n').
printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
Space this out, and perhaps remove some of the superfluous parentheses:
while (i < 1 && i 4);
{
printf("You made wrong choice.\nPlease make a new
choice\n");
scanf("%d",&i);
}
Perhaps you should have set i == 0 initially, and eliminated the first
input section. Or, you could have left i uninitialized and used a do...
....while loop.
switch(i)
{
case 1:
{
insert();
break;
}
case 2:
{
search();
break;
}
case 3:
{
show(list);
break;
}
case 4:
exit();
break;
}
Okay, here's where you need smaller tabs. I've corrected case 4 for you.
Notice also that the braces are unnecessary.
return i;
}
/*In this function the user insert the data.*/
struct sells *insert(void)
{
char titlos[15];
char tragoudisths[15];
int timi;
struct sells *n;
float m_out;
int i;
m_out=0;
printf("%f\n",&m_out);
n=alloc();
printf("Give the title of the CD\n");
scanf("%s",&titlos);
getchar();
scanf ("%s") is very dangerous. Replace with
fgets (titlos, sizeof titlos, stdin);
That will also eliminate the call to getchar() (although you will have
a newline at the end of titlos[] that you may wish to deal with).
>
printf("Give the name of the singer\n");
scanf("%s",&tragoudisths);
getchar();
fgets (tragoudisths, sizeof tragoudisths, stdin);
printf("Give the price of the CD\n");
scanf("%d",&timi);
getchar();
Here scanf() is okay, because you are getting an integer, not a string.
while((timi<0)||(timi>100))
Once again, space and unparentheses:
while (timi < 0 || timi 100)
{
printf("You have insert a wrong price.\nGive a new
price.\n");
I would put two calls to puts() here.
scanf("%d",&timi);
getchar();
}
getchar();
n->tittle=strdup(titlos);
strdup isn't an ANSI C function, and therefore shouldn't be used in clc
code. (Unless you defined it yourself, which I'll know by the end of the
source).
n->singer=strdup(tragoudisths);
n->price=timi;
if(list==NULL)
{
list=alloc();
Much better to write
list = malloc (sizeof *list);
and eliminate the alloc() function altogether.
list->tittle=strdup(n->tittle);
list->singer=strdup(n->singer);
list->price=n->price;
list->next=NULL;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))==0)
{
list->count++;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))<0)
{
n=alloc();
n = malloc (sizeof *n);
n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
n->count=1;
n->next=list;
list=n;
printf("%s\t\t%s\t\t%d\t\t\n",list->tittle,list->singer,list->price);
}
else
{
list->next=insert();
return list;
}
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
exit();
break;
}
Fixed case 2 for you. (I use 2-space tabs, but 3 or 4 is fine.)
return n;
}
/*In this function the user search for an element.Also he can modify
it.
If someone can i would like to tell me what i have to do so as to
delete an element.*/
Make a new function which free()'s the element. free() is the opposite
of malloc. Also, if this is a list of some sort (I'm not looking at
the big picture this early in the thread), you'll need some other
management code.
struct sells *search(void)
{
char titlos[15];
int i,k,out;
char choice;
printf("Give the tittle of the CD that you want to find\n");
puts("Text doesn't need a \n with puts().");
gets(titlos);
Aaaah! gets should /never/, /ever/ be used. There is no possible way to
use it safely.
Use: fgets (titlos, sizeof titlos, stdin);
getchar();
if(list==NULL||(out=strcmp(list->tittle,titlos))>0)
{
printf("The tittle that you want doesn't exist.\n");
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
No braces, smaller indent.
}
return list;
}
else if(out==0)
{
printf("Tittle: \t%s\n",list->tittle);
printf("Singer: \t%s\n",list->singer);
printf("Price: \t%d\n",list->price);
printf("Do you want to make any change;\n");
printf("If Yes press \"1\" else \"0\" to return to
menu\n");
Again, consider `backtick and apostrophe' quotes. (Or, ``double'' quotes).
printf("Make your choice");
scanf("%d",&k);
switch(k)
{
case 1:
{
printf("Do you want ot change the tittle;\nPress Y(YES)
or N(NO)\t");
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new tittle:\n");
gets(list->tittle);
getchar();
}
printf("Do you want ot change the name of the
singer;\nPress Y(YES) or N(NO)\t");
1) Use lowercase.
2) Provide a default case, like "Change singer name (y/[n])? "
This indicates that your two choices are y and n, (usually case
insentive), which obviously mean yes and no, and that if nothing
is entered, n will be chosen.
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new name:\n");
gets(list->singer);
Do /not/ use gets. Use fgets (list->singer, size, stdin) where size will
be determined wherever you allocated memory for list->singer. (You /did/
allocated memory, right?)
getchar();
}
printf("Do you want ot change the price of the
CD;\nPress Y(YES) or N(NO)\t");
(y/[n])
scanf("%s",&choice);
getchar();
if(choice=='Y')
{
printf("Give new price:\n");
scanf("%d",&list->price);
while((list->price<0)&&(list->price>100))
{
printf("You have insert a wrong price.\nGive a new
price.\n");
scanf("%d",&list->price);
}
putchar('\n');
}
break;
}
case 0:
{
menu();
break;
}
Smaller indent, no braces.
}
}
return list;
}
/*Function which shows to the user all the element that has already
insert.
In this functio there is a problem.When i insert more the 3 elements,
these elements doesn't appear to me.I think that the problem is in my
insert
but i am not quite sure.*/
int show(struct sells *list)
{
int i,k;
k=1;
These two lines can be combined: int i, k = 1;
printf("\t\t-------LIST--------\n");
printf("Num\t\tTittle\t\tSinger\t\tPrice\n");
while(list!=NULL)
{
printf("%d\t%s\t\t%s\t\t%d\t\t\n",k,list->tittle,list->singer,list->price);
k++;
list=list->next;
}
printf("What do you want to do\n");
printf("Press \"1\" to retutn to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
}
In case 1, I'm pretty sure that you have indirect recursion. This source is
too big to check on that for sure. If so, you should reconsider your design.
return 0;
}
/*Exit from the programm*/
int exit(void)
{
int i;
printf("Do you really want to exit from the programm;\n");
printf("If Yes press \"1\" else \"0\"\n");
printf("Make your choice");
You need a ": " at the end of that line if you're grabbing input from it.
Otherwise if I type `monkey' I'll see "Make your choicemonkey" and I'll
think, "What's a choicemonkey?".
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
break;
}
In addition to my usual comments, you need a default case here to handle
erroneous input.
return 0;
}
}
--
Andrew Poelstra <http://www.wpsoftware.net/projects/>
To email me, use "apoelstra" at the above domain.
"You people hate mathematics." -- James Harris
Andrew Poelstra said:
<snip>
>struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
No need for the cast. Use
return malloc (sizeof (struct sells));
Better still, either ditch the function entirely, or make it do some useful
work (e.g. initialisation):
struct sells *alloc(void)
{
static const struct sells blank;
struct sells *new = malloc(sizeof *new);
if(new != NULL)
{
*new = blank;
}
return new;
}
(Actually, I question why you're using a function at all for this.)
Quite so. But a use could be found. :-)
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
Andrew Poelstra napsal(a):
On 2006-07-07, de*****@hotmail.com <de*****@hotmail.comwrote:
>Hi.I am started learning Programm language C before some time.I am
trying to make a programm about a very simple "sell shop".This programm
hasn't got any compile problem but when i run it i face some other
ploblems which i can not correct.I would appreciated if someone take a
look at my programm so as to help me.I tried many times to find out
where my mistakes are but i didn't manage something.
I have made some comments due to the programm so as to make it more
easy to as.
Any recommendations about my writing style in the programm are
acceptable.
Please help me!!!
Actually, this is one of the most well-styled programs I've seen posted
here (by a novice) in a while. I recommend you use 2 or 4-space tabs
instead of 8.
Nope, indentation is done by 8-columns wide tabs, period.
>
>Programm
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
/* Functions*/
int menu(void);
struct sells *insert(void);
struct sells *search(void);
int show(struct sells *list);
int exit(void);
/*Struct*/
struct sells{
char *tittle;
char *singer;
int price;
int count;
struct sells *next;
};
struct sells *list;
struct sells *alloc(void)
{
return(struct sells*)malloc(sizeof(struct sells));
No need for the cast. Use
return malloc (sizeof (struct sells));
You haven't read the thread, have you?
>
(Actually, I question why you're using a function at all for this.)
>}
/*Function main*/
int main(void)
{
menu();
return 0;
}
Even in small functions, formatting is essential.
>/*Function menu.In this function the user has the opportunity to choose
what he/she want to do*/
int menu(void)
{
int i;
printf("---------MENU---------\n");
printf("Press \"1\" to insert data\n");
printf("Press \"2\" to search data\n");
printf("Press \"3\" to show data\n");
printf("Press \"4\" to exit from programm\n");
It would perhaps be more efficient (and easier to read) if you replaced
printf("...\n"); with puts("...");
again...
>
Notice how I eliminated the \n that way. Also, you might want to replace
your \"escaped double quotes\" with ``manual double quotes'' (notice how
they bend in the right direction).
> putchar('\n\n');
This isn't correct. You can put two characters into single quotes. You'll
need to do printf ("\n\n"), two puts("") or two putchar ('\n').
....
>
> printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
Space this out, and perhaps remove some of the superfluous parentheses:
while (i < 1 && i 4);
semicolon?
>
> {
printf("You made wrong choice.\nPlease make a new
choice\n");
scanf("%d",&i);
}
Perhaps you should have set i == 0 initially, and eliminated the first
input section. Or, you could have left i uninitialized and used a do...
...while loop.
> switch(i)
{
case 1:
{
insert();
break;
}
case 2:
{
search();
break;
}
case 3:
{
show(list);
break;
}
case 4:
exit();
break;
}
Okay, here's where you need smaller tabs. I've corrected case 4 for you.
Ugly as hell.
switch (whatever) {
case 1:
whatever
break;
}
is one of correct indentation.
Notice also that the braces are unnecessary.
> return i;
}
/*In this function the user insert the data.*/
struct sells *insert(void)
{
char titlos[15];
char tragoudisths[15];
int timi;
struct sells *n;
float m_out;
int i;
m_out=0;
printf("%f\n",&m_out);
n=alloc();
printf("Give the title of the CD\n");
scanf("%s",&titlos);
getchar();
scanf ("%s") is very dangerous. Replace with
fgets (titlos, sizeof titlos, stdin);
and again...
That will also eliminate the call to getchar() (although you will have
a newline at the end of titlos[] that you may wish to deal with).
> printf("Give the name of the singer\n");
scanf("%s",&tragoudisths);
getchar();
fgets (tragoudisths, sizeof tragoudisths, stdin);
> printf("Give the price of the CD\n");
scanf("%d",&timi);
getchar();
Here scanf() is okay, because you are getting an integer, not a string.
> while((timi<0)||(timi>100))
Once again, space and unparentheses:
while (timi < 0 || timi 100)
> {
printf("You have insert a wrong price.\nGive a new
price.\n");
I would put two calls to puts() here.
> scanf("%d",&timi);
getchar();
}
getchar();
n->tittle=strdup(titlos);
strdup isn't an ANSI C function, and therefore shouldn't be used in clc
code. (Unless you defined it yourself, which I'll know by the end of the
source).
> n->singer=strdup(tragoudisths);
n->price=timi;
if(list==NULL)
{
list=alloc();
Much better to write
list = malloc (sizeof *list);
and eliminate the alloc() function altogether.
> list->tittle=strdup(n->tittle);
list->singer=strdup(n->singer);
list->price=n->price;
list->next=NULL;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))==0)
{
list->count++;
printf("%f",&m_out);
}
else if((m_out=(strcmp(titlos,n->tittle)))<0)
{
n=alloc();
n = malloc (sizeof *n);
> n->tittle=strdup(titlos);
n->singer=strdup(tragoudisths);
n->price=timi;
n->count=1;
n->next=list;
list=n;
printf("%s\t\t%s\t\t%d\t\t\n",list->tittle,list->singer,list->price);
}
else
{
list->next=insert();
return list;
}
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
exit();
break;
}
Fixed case 2 for you. (I use 2-space tabs, but 3 or 4 is fine.)
Blaah. Mixing of variable length indent is Evil(TM).
Read linux/Documentation/CodingStyle...
>
> return n;
}
/*In this function the user search for an element.Also he can modify
it.
If someone can i would like to tell me what i have to do so as to
delete an element.*/
Make a new function which free()'s the element. free() is the opposite
of malloc. Also, if this is a list of some sort (I'm not looking at
the big picture this early in the thread), you'll need some other
management code.
>struct sells *search(void)
{
char titlos[15];
int i,k,out;
char choice;
printf("Give the tittle of the CD that you want to find\n");
puts("Text doesn't need a \n with puts().");
Moreover it's not parsed...
>
> gets(titlos);
Aaaah! gets should /never/, /ever/ be used. There is no possible way to
use it safely.
Use: fgets (titlos, sizeof titlos, stdin);
> getchar();
if(list==NULL||(out=strcmp(list->tittle,titlos))>0)
{
printf("The tittle that you want doesn't exist.\n");
printf("Press \"1\" to return to menu or \"2\" to exit\n");
printf("Make your choice");
scanf("%d",&i);
switch(i)
{
case 1:
{
menu();
break;
}
case 2:
{
exit();
break;
}
No braces, smaller indent.
....
--
js
Andrew Poelstra said:
<snip>
>
> printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
Space this out, and perhaps remove some of the superfluous parentheses:
while (i < 1 && i 4);
That's the most efficient while loop I've seen in some time. Not content
with having the smallest possible loop body, it carefully avoids ever
executing it!
More prosaically: if the LHS of the && is true, the RHS cannot be true, so
the condition will always fail.
<snip>
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
Jiri Slaby said:
<snip>
>
Coding style? yes, try to use indent in way such this one:
indent -npro -kr -i8 -ts8 -sob -l80 -ss -ncs
(un*x utility for indentation)
There's a lot more to coding style than layout.
And anyway, you're using the wrong settings! :-)
me@here:~cat .indent.pro | fmt
--blank-lines-after-declarations --blank-lines-after-procedures
--blank-lines-before-block-comments --braces-after-if-line
--brace-indent0 --braces-after-struct-decl-line --blank-before-sizeof
--case-brace-indentation0 --continuation-indentation2 --case-indentation2
--break-function-decl-args --honour-newlines --indent-level2
--line-length72 --comment-line-length64 --continue-at-parentheses
--break-after-boolean-operator --dont-cuddle-do-while --dont-cuddle-else
--no-space-after-casts --no-space-after-function-call-names
--no-space-after-parentheses --dont-break-procedure-type
--no-space-after-for --no-space-after-if --no-space-after-while
--dont-space-special-semicolon -nut --preserve-mtime
--struct-brace-indentation0
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
<de*****@hotmail.comwrote:
I had a bit more time so I took another look.
struct sells *list;
There that little devil is! It's global, that's considered bad form but I
could live with that in this instance. But you need to initialize it to
NULL! The code in the sequel depends on that!
I still think the best thing to do is start over. If you follow all the
tweaking and formatting and syntax bitching in this thread and fix all that
up; I would guess the chances of your program working, after all that
effort, at about 1000:1. This is a pretty elementary error.
Skarmander wrote:
Richard Heathfield wrote:
>jaysome said:
>>On Fri, 07 Jul 2006 09:10:14 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
<snip>
>>What other Standard C functions would be good candidates for the
"-function" option?
Just gets(). But I'd flag a few others as being "experts-only"
functions, which you should not use in "real" code unless you know
/exactly/ what you're doing:
<snip>
>From <stdlib.h>
[...] calloc, malloc, realloc, free, [...]
Let's get this straight: you shouldn't dynamically allocate memory in C
unless you're an expert?
It is extremely easy to get wrong.
Well, actually, no arguments there -- but why not just say C is for
experts and leave it at that, then? For most applications, you need to
be an expert to write a program *without* dynamic memory allocation!
Well, in significant parts of the embedded world you are explicitly
banned from using allocated memory, and I wrote a *lot* of code without
any difficulty with that restriction even when I was new to programming.
For example, the first significant program I ever wrote (which I wrote
in BASIC) was "The Game of Life". http://www.google.com/search?sourcei...ame+of+life%22
If you have a static board size there is absolutely no need for dynamic
memory allocation. IIRC it was interesting to implement.
There are plenty of other significant projects one can implement without
dynamic memory allocation. Sometimes by having a reasonable static size
and reporting if the user tries to go beyond it, sometimes without any
need at all for dynamic memory allocation.
--
Flash Gordon, living in interesting times.
Web site - http://home.flash-gordon.me.uk/
comp.lang.c posting guidelines and intro: http://clc-wiki.net/wiki/Intro_to_clc
osmium said:
<de*****@hotmail.comwrote:
I had a bit more time so I took another look.
>struct sells *list;
There that little devil is! It's global, that's considered bad form but I
could live with that in this instance. But you need to initialize it to
NULL!
He did - by putting it at file scope. That was the mistake he didn't make.
He made almost all the others, though. :-)
I would guess the chances of your program working, after all that
effort, at about 1000:1. This is a pretty elementary error.
It doesn't compile here, so I reckon the probability of it working is 0.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
osmium wrote:
<de*****@hotmail.comwrote:
I had a bit more time so I took another look.
>struct sells *list;
There that little devil is! It's global, that's considered bad form but I
could live with that in this instance. But you need to initialize it to
NULL! The code in the sequel depends on that!
If, as you say, it is a global, then the C standard guarantees that it
is initialised to a null pointer.
I still think the best thing to do is start over. If you follow all the
tweaking and formatting and syntax bitching in this thread and fix all that
up; I would guess the chances of your program working, after all that
effort, at about 1000:1. This is a pretty elementary error.
Nope, yours is the elementary error. deppy_3 would be better off
following the advice of those who know C better than you, such as
Richard Heathfield.
--
Flash Gordon, living in interesting times.
Web site - http://home.flash-gordon.me.uk/
comp.lang.c posting guidelines and intro: http://clc-wiki.net/wiki/Intro_to_clc
Richard Heathfield napsal(a):
Jiri Slaby said:
<snip>
>Coding style? yes, try to use indent in way such this one:
indent -npro -kr -i8 -ts8 -sob -l80 -ss -ncs
(un*x utility for indentation)
There's a lot more to coding style than layout.
And anyway, you're using the wrong settings! :-)
Nice, but this is Linux kernel default, see linux/scripts/Lindent ;).
--
js
"Richard Bos" <rl*@hoekstra-uitgeverij.nlwrote in message
news:44****************@news.xs4all.nl...
jaysome <ja*****@spamcop.netwrote:
[snip]
>What other Standard C functions would be good candidates for the
"-function" option?
None, really. gets() is the only one that can _not_ be used safely.
Functions like scanf() are easy to use unsafely, but can be made safe in
a simple, though WOMBATty, way, by constructing the format string at run
time, using something like (warning: not tested!)
sprintf(format_string, "%%%ds", current_size_of_buffer);
scanf(format_string, destination_string);
I would add strtok(), which is hopelessly broken. The POSIX strtok_r() is
the correct way to write a simple token parser.
Jiri Slaby <ge*@wo.czwrites:
Andrew Poelstra napsal(a):
[...]
>Actually, this is one of the most well-styled programs I've seen posted
here (by a novice) in a while. I recommend you use 2 or 4-space tabs
instead of 8.
Nope, indentation is done by 8-columns wide tabs, period.
Nope.
[...]
>Fixed case 2 for you. (I use 2-space tabs, but 3 or 4 is fine.)
Blaah. Mixing of variable length indent is Evil(TM).
Read linux/Documentation/CodingStyle...
I might consider reading that, and I'd certainly follow it if I were
working on the Linux kernel. But there are a number of different
layout styles and there's no objective reason to think that indenting
with 8-column tabs is better than any other style.
BTW, if you want people to read linux/Documentation/CodingStyle, you
should provide an actual link.
(Personally, I use 4-space indentation and avoid hard tab characters
altogether, but I'm not going to claim that I'm right and everyone
else is wrong.)
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
Keith Thompson <ks***@mib.orgwrites:
Jiri Slaby <ge*@wo.czwrites:
Andrew Poelstra napsal(a):
[...]
><snip>
BTW, if you want people to read linux/Documentation/CodingStyle, you
should provide an actual link.
8 spaces tabs feel awkward to me. I find it hard to read code indented
that way because I'm not used to it.
Here's a snippet from the document Jiri talks about :
Chapter 1: Indentation
Tabs are 8 characters, and thus indentations are also 8 characters.
There are heretic movements that try to make indentations 4 (or even
2!)
characters deep, and that is akin to trying to define the value of PI
to
be 3.
Rationale: The whole idea behind indentation is to clearly define where
a block of control starts and ends. Especially when you've been
looking
at your screen for 20 straight hours, you'll find it a lot easier to
see
how the indentation works if you have large indentations.
Now, some people will claim that having 8-character indentations makes
the code move too far to the right, and makes it hard to read on a
80-character terminal screen. The answer to that is that if you need
more than 3 levels of indentation, you're screwed anyway, and should
fix
your program.
In short, 8-char indents make things easier to read, and have the
added
benefit of warning you when you're nesting your functions too deep.
Heed that warning.
Don't put multiple statements on a single line unless you have
something to hide:
if (condition) do_this;
do_something_everytime;
Outside of comments, documentation and except in Kconfig, spaces are
never
used for indentation, and the above example is deliberately broken.
Get a decent editor and don't leave whitespace at the end of lines.
On 2006-07-07, Jiri Slaby <ge*@wo.czwrote:
Andrew Poelstra napsal(a):
>Actually, this is one of the most well-styled programs I've seen posted
here (by a novice) in a while. I recommend you use 2 or 4-space tabs
instead of 8.
Nope, indentation is done by 8-columns wide tabs, period.
Have you seen some of the programs posted here? Most have /no/ indentation.
>No need for the cast. Use
return malloc (sizeof (struct sells));
You haven't read the thread, have you?
According to my newserver, I was the first person to actually comment
on the entire code. I've no idea what you mean.
>It would perhaps be more efficient (and easier to read) if you replaced
printf("...\n"); with puts("...");
again...
Please give me a link to the last post where this was mentioned.
>> printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
Space this out, and perhaps remove some of the superfluous parentheses:
while (i < 1 && i 4);
semicolon?
Oops! My mistake.
>> case 4:
exit();
break;
}
Okay, here's where you need smaller tabs. I've corrected case 4 for you.
Ugly as hell.
switch (whatever) {
case 1:
whatever
break;
}
is one of correct indentation.
1) Tabs are never correct indentation.
2) Aside from that, there is no "correct" indentation. 2, 3, or 4-space
is fine. 8 is rarely readable for real code.
>scanf ("%s") is very dangerous. Replace with
fgets (titlos, sizeof titlos, stdin);
and again...
Again what? I detect a AOL "Me too" posting style.
>> case 2:
exit();
break;
}
Fixed case 2 for you. (I use 2-space tabs, but 3 or 4 is fine.)
Blaah. Mixing of variable length indent is Evil(TM).
Read linux/Documentation/CodingStyle...
"Mixing of variable length indent"? Please tell me what that means
and where I did it.
>> printf("Give the tittle of the CD that you want to find\n");
puts("Text doesn't need a \n with puts().");
Moreover it's not parsed...
It's an output statement. You don't "parse" output. Unless you meant
spellchecked or proofread...
--
Andrew Poelstra <http://www.wpsoftware.net/projects/>
To email me, use "apoelstra" at the above domain.
"You people hate mathematics." -- James Harris
Keith Thompson said:
<snip>
>
(Personally, I use 4-space indentation and avoid hard tab characters
altogether, but I'm not going to claim that I'm right and everyone
else is wrong.)
Good, because such a claim would be (very slightly) inaccurate. ;-)
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
On 2006-07-07, Richard Heathfield <in*****@invalid.invalidwrote:
Andrew Poelstra said:
<snip>
>>
>> printf("Make your choice\t");
scanf("%d",&i);
while((i<1)&&(i>4))
Space this out, and perhaps remove some of the superfluous parentheses:
while (i < 1 && i 4);
That's the most efficient while loop I've seen in some time. Not content
with having the smallest possible loop body, it carefully avoids ever
executing it!
Thanks! :-)
More prosaically: if the LHS of the && is true, the RHS cannot be true, so
the condition will always fail.
You are correct. The use of "gets" and the malloc wrapper caused me to
go through and correct style and UB issues while ignoring logical ones;
there were quite a few such issues I deliberately ignored (the post was
already 400 lines), and I wouldn't doubt that I missed many more.
I have to wonder how the posted code actually ran.
--
Andrew Poelstra <http://www.wpsoftware.net/projects/>
To email me, use "apoelstra" at the above domain.
"You people hate mathematics." -- James Harris
Andrew Poelstra said:
On 2006-07-07, Jiri Slaby <ge*@wo.czwrote:
<snip>
>>
You haven't read the thread, have you?
According to my newserver, I was the first person to actually comment
on the entire code.
According to mine, my compiler was the first to comment on the entire code.
Beat you by 6 hours 19 minutes. :-)
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
Flash Gordon wrote:
Skarmander wrote:
>Richard Heathfield wrote:
>>jaysome said:
On Fri, 07 Jul 2006 09:10:14 +0000, Richard Heathfield
<in*****@invalid.invalidwrote:
<snip>
What other Standard C functions would be good candidates for the
"-function" option?
Just gets(). But I'd flag a few others as being "experts-only"
functions, which you should not use in "real" code unless you know
/exactly/ what you're doing:
<snip>
>>From <stdlib.h>
[...] calloc, malloc, realloc, free, [...]
Let's get this straight: you shouldn't dynamically allocate memory in
C unless you're an expert?
It is extremely easy to get wrong.
>Well, actually, no arguments there -- but why not just say C is for
experts and leave it at that, then? For most applications, you need to
be an expert to write a program *without* dynamic memory allocation!
Well, in significant parts of the embedded world you are explicitly
banned from using allocated memory, and I wrote a *lot* of code without
any difficulty with that restriction even when I was new to programming.
Oh, let's get *that* straight too: obviously it's actually very *easy* to
write code if you can't use more than, say, 42 memory units exactly -- or
tell people such code can't be written.
For many applications that's simply not good enough, though. You'll want a
program that can scale to core without having to recompile it.
If you want a program that has to utilize those 42 units of memory to the
best of its ability, it's not unthinkable that you'll end up implementing
(pseudo-)dynamic memory allocation yourself.
For example, the first significant program I ever wrote (which I wrote
in BASIC) was "The Game of Life".
http://www.google.com/search?sourcei...ame+of+life%22
If you have a static board size there is absolutely no need for dynamic
memory allocation. IIRC it was interesting to implement.
The key point being "if you have a static board size". And for Life, that's
indeed not a significant restriction.
There are plenty of other significant projects one can implement without
dynamic memory allocation.
This I do not dispute.
Sometimes by having a reasonable static size and reporting if the user
tries to go beyond it, sometimes without any need at all for dynamic
memory allocation.
But the latter class is a minority, and the former a distinct nuisance. I
cannot recount the number of times a program (quite likely written in C)
used a "reasonable" static size and refused my input. The programmer
responsible would probably shug their shoulders, increase the #define'd
constants a bit and recompile. Problem solved! For now.
S.
Richard Heathfield <in*****@invalid.invalidwrites:
Keith Thompson said:
<snip>
>>
(Personally, I use 4-space indentation and avoid hard tab characters
altogether, but I'm not going to claim that I'm right and everyone
else is wrong.)
Good, because such a claim would be (very slightly) inaccurate. ;-)
Good point. A more accurate claim would be that I'm right and
everyone who disagrees with me is wrong. 8-)}
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
Boehm's garbage collector is delivered as standard in lcc-win32,
but you can find it for most compilers or environments.
jacob
Richard Bos wrote:
jaysome <ja*****@spamcop.netwrote:
>>One of the warnings, 421, flagged gets() as being dangerous. The
PC-lint text documentation (MSG.TXT), which you can download from
Gimpel's web site, says this about warning 421:
421 Caution -- function 'Symbol' is considered dangerous -- This
message is issued (by default) for the built-in function gets.
This function is considered dangerous because there is no
mechanism to ensure that the buffer provided as first argument
will not overflow. A well known computer virus (technically a
worm) was created based on this defect. Through the -function
option, the user may designate other functions as dangerous.
What other Standard C functions would be good candidates for the
"-function" option?
None, really. gets() is the only one that can _not_ be used safely.
That is not the opinion of the C standards comitee.
We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
None of the comitee members cared to contradict him or express another
opinion.
Look for the thread
"Why does rewind() ignore errors"?
initiated by K. Thompson
Skarmander said:
Flash Gordon wrote:
>Skarmander wrote:
<snip>
>>Let's get this straight: you shouldn't dynamically allocate memory in
C unless you're an expert?
It is extremely easy to get wrong.
<snip>
>
For many applications that's simply not good enough, though. You'll want a
program that can scale to core without having to recompile it.
That is an excellent reason to learn how to do dynamic memory allocation
correctly. It is a very poor reason for using dynamic memory allocation
without having first learned how to do it correctly.
Dynamic memory allocation is indeed a vital technique. Let's do it right.
<snip>
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
jacob navia" <ja***@jacob.remcomp.frwrote in message
news:44*********************@news.orange.fr...
Richard Bos wrote:
>jaysome <ja*****@spamcop.netwrote:
>>>One of the warnings, 421, flagged gets() as being dangerous. The
PC-lint text documentation (MSG.TXT), which you can download from
Gimpel's web site, says this about warning 421:
421 Caution -- function 'Symbol' is considered dangerous -- This
message is issued (by default) for the built-in function gets.
This function is considered dangerous because there is no
mechanism to ensure that the buffer provided as first argument
will not overflow. A well known computer virus (technically a
worm) was created based on this defect. Through the -function
option, the user may designate other functions as dangerous.
What other Standard C functions would be good candidates for the
"-function" option?
None, really. gets() is the only one that can _not_ be used safely.
That is not the opinion of the C standards comitee.
We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
None of the comitee members cared to contradict him or express another
opinion.
Look for the thread
"Why does rewind() ignore errors"?
initiated by K. Thompson
Right, all you have to do to use it safely is to reopen standard input as a
file, do some seeks and voilla! The gets() function can be used safely.
It's sort of like saying: "You CAN roast marshmallows safely with hydrogen
bombs -- you just have to stand the right distance away."
Richard Heathfield wrote:
Skarmander said:
>Flash Gordon wrote:
>>Skarmander wrote:
<snip>
>>>Let's get this straight: you shouldn't dynamically allocate memory in
C unless you're an expert?
It is extremely easy to get wrong.
<snip>
>For many applications that's simply not good enough, though. You'll want a
program that can scale to core without having to recompile it.
That is an excellent reason to learn how to do dynamic memory allocation
correctly. It is a very poor reason for using dynamic memory allocation
without having first learned how to do it correctly.
Dynamic memory allocation is indeed a vital technique. Let's do it right.
And? And? "Hey, let's be careful out there."
S.
Keith Thompson said:
Richard Heathfield <in*****@invalid.invalidwrites:
>Keith Thompson said:
<snip>
>>>
(Personally, I use 4-space indentation and avoid hard tab characters
altogether, but I'm not going to claim that I'm right and everyone
else is wrong.)
Good, because such a claim would be (very slightly) inaccurate. ;-)
Good point. A more accurate claim would be that I'm right and
everyone who disagrees with me is wrong. 8-)}
Close, but no banana. :-)
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
jacob navia said:
Richard Bos wrote:
<snip>
>gets() is the only one that can _not_ be used safely.
That is not the opinion of the C standards comitee.
It is certainly possible to envisage a situation where a C programmer could
use gets() without the system being compromised - for example, he writes
the program, he compiles it, runs it himself on a single-user system, types
the input himself, and then deletes the program immediately when he has the
results he requires. This paragraph should not be taken as an endorsement
of gets(), which I maintain cannot be used safely, despite my previous
sentence. The programmer might decide against deleting the program "in case
it comes in handy again". He might be distracted by other matters, and
forget to delete the program. And so on.
We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
The ISO C Committee writes the Standard, and the Standard defines the
language. But the Standard has no authority to /require/ programmers to use
gets(). If the Committee are daft enough to leave gets() in the Standard,
that's their problem. We don't have to be daft enough to use it.
A standard C library that omitted gets() would, in my opinion, still be
conforming to all intents and purposes, since a strictly conforming program
dare not trust gets(), and therefore dare not use it, in case the user
invokes undefined behaviour. The omission could be viewed as a sort of
extension. :-)
None of the comitee members cared to contradict him or express another
opinion.
Presumably you mean none of the committee members who happened to read that
thread.
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
jacob navia wrote:
The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
Please. We're having a pleasant academic argument, and do not wish to be
disturbed by all sorts of realistic solutions. :-)
Sure, while you're adding a garbage collector, link in a safe string library
too, and you've probably covered most C mishaps. (I'd think out-of-bounds
errors and failure to check error returns account for the remainder.) The
context of the discussion was a "vanilla" C implementation, though, with
standard functions considered dangerous. The standard doesn't prohibit a
garbage collecting implementation, but doesn't guarantee it either.
S.
On Fri, 7 Jul 2006 15:49:56 GMT, in comp.lang.c , Jiri Slaby
<ge*@wo.czwrote:
>
Nope, indentation is done by 8-columns wide tabs, period.
Apparently you haven't worked with many large programmes...
--
Mark McIntyre
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it."
--Brian Kernighan
jacob navia <ja***@jacob.remcomp.frwrites:
[...]
That is not the opinion of the C standards comitee.
We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
None of the comitee members cared to contradict him or express another
opinion.
I'm not sure that that's correct, though it may be. A number of
people have participated in that thread, and many (perhaps most) of
them expressed opinions that differed from Doug Gywn's. I don't know
who is on the committee and who isn't. It's entirely possible that
someone who disagreed with Doug Gwyn is also on the committee.
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
Skarmander <in*****@dontmailme.comwrites:
jacob navia wrote:
>The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
Please. We're having a pleasant academic argument, and do not wish to
be disturbed by all sorts of realistic solutions. :-)
Sure, while you're adding a garbage collector, link in a safe string
library too, and you've probably covered most C mishaps. (I'd think
out-of-bounds errors and failure to check error returns account for
the remainder.) The context of the discussion was a "vanilla" C
implementation, though, with standard functions considered
dangerous. The standard doesn't prohibit a garbage collecting
implementation, but doesn't guarantee it either.
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
That's not to say that it's a good idea for a program to do something
like that, and I wouldn't object strongly if a future revision of the
C standard made that kind of thing undefined behavior, but it is
something to keep in mind.
Also, I don't believe it's possible to implement garbage collection in
portable C. A garbage collector has to be able to detect any
currently active pointers, which means it has to be able to examine
all active automatic, static, and allocated objects (even ignoring the
issue of pointers being stashed somewhere else). I believe there are
existing garbage collectors that do a good job of this on many
existing systems, but they do so with tricks that are not strictly
portable. (I'm not an expert on garbage collection; please correct me
if I'm mistaken.)
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
Keith Thompson said:
[...] I don't believe it's possible to implement garbage collection in
portable C.
And in any case, memory management is far too important to be left to "the
system". (But cf. Lisp!)
--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999 http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)
Keith Thompson wrote:
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
Yes, that will not work. Pointers must be left around so that the GC
sees them, what is not very difficult to do. Just do nothing and it
will work !
That's not to say that it's a good idea for a program to do something
like that, and I wouldn't object strongly if a future revision of the
C standard made that kind of thing undefined behavior, but it is
something to keep in mind.
Yes. It was usual for windows programmers to store pointers in the
"window extrabytes", of the window object.
That would not work.
Also, I don't believe it's possible to implement garbage collection in
portable C. A garbage collector has to be able to detect any
currently active pointers, which means it has to be able to examine
all active automatic, static, and allocated objects (even ignoring the
issue of pointers being stashed somewhere else). I believe there are
existing garbage collectors that do a good job of this on many
existing systems, but they do so with tricks that are not strictly
portable. (I'm not an expert on garbage collection; please correct me
if I'm mistaken.)
You can't write many things in C and you need assembly. For instance
to write fe_getenv() you need assembly since acccesing the FPU is
impossible in C.
That is normal practice for the standard library, and it is the same for
the gc library.
In a similar vein, Intel proposes the "standard math kernel" library,
and it is mostly highmly optimized assembly code.
Keith Thompson wrote:
Skarmander <in*****@dontmailme.comwrites:
>>jacob navia wrote:
>>>The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
Please. We're having a pleasant academic argument, and do not wish to
be disturbed by all sorts of realistic solutions. :-)
Sure, while you're adding a garbage collector, link in a safe string
library too, and you've probably covered most C mishaps. (I'd think
out-of-bounds errors and failure to check error returns account for
the remainder.) The context of the discussion was a "vanilla" C
implementation, though, with standard functions considered
dangerous. The standard doesn't prohibit a garbage collecting
implementation, but doesn't guarantee it either.
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
Not nit picking, just curious: how?
--
Ian Collins.
Dann Corbit wrote:
jacob navia" <ja***@jacob.remcomp.frwrote in message
We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
None of the comitee members cared to contradict him or express another
opinion.
Look for the thread
"Why does rewind() ignore errors"?
initiated by K. Thompson
Right, all you have to do to use it safely is to reopen standard input as a
file, do some seeks and voilla! The gets() function can be used safely.
It's sort of like saying: "You CAN roast marshmallows safely with hydrogen
bombs -- you just have to stand the right distance away."
I don't get what you mean. Can you please explain? Was it a joke?
use fseek()? to where?
Sorry for all the questions.
/* Open stdin as a file. Possibly a piss-take */
#include <stdio.h>
int main(void)
{
FILE *f = fopen(stdin, "r");
if (f == NULL)
return 1;
char buf[10];
for (int i=0; i<10; i++)
buf[i] = fgetc(f);
printf("%s\n", buf);
fclose(f);
return 0;
}
Was it a joke?
Ian Collins wrote:
Keith Thompson wrote:
>Skarmander <in*****@dontmailme.comwrites:
>>jacob navia wrote:
The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
Please. We're having a pleasant academic argument, and do not wish to
be disturbed by all sorts of realistic solutions. :-)
Sure, while you're adding a garbage collector, link in a safe string
library too, and you've probably covered most C mishaps. (I'd think
out-of-bounds errors and failure to check error returns account for
the remainder.) The context of the discussion was a "vanilla" C
implementation, though, with standard functions considered
dangerous. The standard doesn't prohibit a garbage collecting
implementation, but doesn't guarantee it either.
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
Not nit picking, just curious: how?
Copy the bytes of the pointer to an array of unsigned char. Copy back when
done. The standard requires this to work. (The first stage, at least. I must
admit I haven't looked closely at the guarantees for the second stage.)
This is not likely to occur in a real program. Many if not most other
pointer tricks that defeat GC are not strictly conforming, like the
notorious "XOR two pointer values to implement a doubly linked list" trick.
Many other tricks break the aliasing rules and aren't conforming either.
S.
Skarmander wrote:
Ian Collins wrote:
>Keith Thompson wrote:
>>>
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
Not nit picking, just curious: how?
Copy the bytes of the pointer to an array of unsigned char. Copy back
when done. The standard requires this to work. (The first stage, at
least. I must admit I haven't looked closely at the guarantees for the
second stage.)
Yuck!
This is not likely to occur in a real program. Many if not most other
pointer tricks that defeat GC are not strictly conforming, like the
notorious "XOR two pointer values to implement a doubly linked list"
trick. Many other tricks break the aliasing rules and aren't conforming
either.
I thought so, I've used a GC library with C before and hadn't run into
any problems.
--
Ian Collins.
Skarmander <in*****@dontmailme.comwrites:
Ian Collins wrote:
>Keith Thompson wrote:
>>Skarmander <in*****@dontmailme.comwrites:
jacob navia wrote:
The solution to many of this problems of memory allocation is to
allocate dynamically all the memory you need, never call free()
and link with the GARBAGE COLLECTOR.
>
Please. We're having a pleasant academic argument, and do not wish to
be disturbed by all sorts of realistic solutions. :-)
Sure, while you're adding a garbage collector, link in a safe string
library too, and you've probably covered most C mishaps. (I'd think
out-of-bounds errors and failure to check error returns account for
the remainder.) The context of the discussion was a "vanilla" C
implementation, though, with standard functions considered
dangerous. The standard doesn't prohibit a garbage collecting
implementation, but doesn't guarantee it either.
Garbage collection can break some correct C code. A garbage collector
must avoid deallocating anything that the program is using or may use.
A conforming C program could stash away a pointer value in some manner
that can't be detected by the garbage collector, and later retrieve
and dereference it.
Not nit picking, just curious: how?
Copy the bytes of the pointer to an array of unsigned char. Copy back
when done. The standard requires this to work. (The first stage, at
least. I must admit I haven't looked closely at the guarantees for the
second stage.)
Simply copying the bytes of a pointer to an array of unsigned char
isn't likely to fool a garbage collector, unless the garbage collector
makes assumptions about the types of objects (which would require a
great deal of chumminess with the compiler).
My understanding is that GC works by scanning memory for things that
look like pointers, and marking anything pointed to by those pointers
as in-use and therefore not available for deallocation. This can
result in some false positives, if something that's not a pointer
happens to contain a bit representation that matches a valid pointer
value. As far as I know, this isn't a major problem in practice.
Copying the bytes of a pointer *and re-ordering them* would fool the
garbage collector (as long as the object from which you copied them is
destroyed).
I can imagine an application that, for security reasons, keeps certain
data structures encrypted most of the time, decrypting them only when
necessary.
I can also imagine an application storing pointers in a file, to be
retrieved later during the same execution of the same program. This
isn't likely to be useful unless you've built some very large
pointer-intensive data structures, and you don't have enough resources
to keep them all in memory all the time.
I presume most garbage collectors provide a mechanism to temporarily
or permanently disable them, either completely or for a given data
structure. A program that does the kind of exotic pointer
manipulation I describe *and* that needs to work in a
garbage-collecting environment would have to take advantage of this
feature.
Also, garbage collection can be impractical in some environments. A
real-time system can't afford to wait for the GC system to analyze
memory while it's trying to meet a hard deadline.
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
ballpointpenthief wrote:
Dann Corbit wrote:
>jacob navia" <ja***@jacob.remcomp.frwrote in message
>>We had an animated discussion in comp.std.c about why gets() is
still in the standard, and a certain Mr Gwyn, apparently from the
comitee or very associated with them defended gets() with all
kind of weird arguments.
None of the comitee members cared to contradict him or express another
opinion.
Look for the thread
"Why does rewind() ignore errors"?
initiated by K. Thompson
Right, all you have to do to use it safely is to reopen standard input as a
file, do some seeks and voilla! The gets() function can be used safely.
It's sort of like saying: "You CAN roast marshmallows safely with hydrogen
bombs -- you just have to stand the right distance away."
I don't get what you mean. Can you please explain? Was it a joke?
use fseek()? to where?
Sorry for all the questions.
/* Open stdin as a file. Possibly a piss-take */
#include <stdio.h>
int main(void)
{
FILE *f = fopen(stdin, "r");
if (f == NULL)
return 1;
char buf[10];
for (int i=0; i<10; i++)
buf[i] = fgetc(f);
printf("%s\n", buf);
fclose(f);
return 0;
}
You misunderstand something central. stdin is of type FILE*. stdin is
already open when your program starts.
The first argument to fopen is const char*. stdin cannot be that.
You need more practice.
>
Was it a joke?
Yes.
--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein --- This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics
by: Toni |
last post by:
How can I insert a Backgroundimage in my C++ programm?
Microsoft Visual Studio C++, Win 32 Konsolenanwendung.
Can you answer me in germany, because my english is very bad.
An example where very...
|
by: Toni |
last post by:
How can I insert a Backgroundimage in my C++ programm?
Microsoft Visual Studio C++, Win 32 Konsolenanwendung.
Can you answer me in germany, because my english is very bad.
An example where very...
|
by: gregsands |
last post by:
Hi Im trying to install PHP 5.05 on Windows XP running Apache 2.0.54
Apache is running fine and ive made all the changes to the pnp.ini file
and Apache config file, restarted Apache but all I get...
|
by: Alexander F?hrmann |
last post by:
Grüß euch!
Wir haben hier in der Firma ein Problem mit Access!
Sobald man im Access den VBA-Editor aufrufen will, kommt folgender
fehler!
--------------------
Runtime Error!
|
by: sipetar |
last post by:
I have two file.
file looks like this:
first file:
A#b1#c1
B#b2#c2
B#b3
C#b4#c4
D#b5#c5
|
by: Sven |
last post by:
My current application is programmed in Visual Basic 6.0
It is a server and uses the winsock-control
Now I have learned that the winsock control is not (fully) supported by
VB.NET and that I...
|
by: Jan |
last post by:
Hello!
I am looking for a way to do a search&replace in ASCII-Files by a
vb.net 2005 programm.
Of coarse I can open the files, loop to every line, make a replace, and
save the line.
But I wonder...
|
by: shahrazad78 |
last post by:
Hello every body ,
I need help in c/c++ in compiling
I use linux , kosole editor to write a programm in c/c++ .
And I have also gl/glu/glut libraries to use the function of opengl ,and I write a...
|
by: dreiko466 |
last post by:
(sorry about my english...)
I am a newbie in C (3 month expierience)
I have wrote a simple test programm in VS2005, what i do wrong?Please...
In this programm i create a double linked list.Then ...
|
by: taylorcarr |
last post by:
A Canon printer is a smart device known for being advanced, efficient, and reliable. It is designed for home, office, and hybrid workspace use and can also be used for a variety of purposes. However,...
|
by: aa123db |
last post by:
Variable and constants
Use var or let for variables and const fror constants.
Var foo ='bar';
Let foo ='bar';const baz ='bar';
Functions
function $name$ ($parameters$) {
}
...
|
by: ryjfgjl |
last post by:
If we have dozens or hundreds of excel to import into the database, if we use the excel import function provided by database editors such as navicat, it will be extremely tedious and time-consuming...
|
by: Sonnysonu |
last post by:
This is the data of csv file
1 2 3
1 2 3
1 2 3
1 2 3
2 3
2 3
3
the lengths should be different i have to store the data by column-wise with in the specific length.
suppose the i have to...
|
by: Hystou |
last post by:
There are some requirements for setting up RAID:
1. The motherboard and BIOS support RAID configuration.
2. The motherboard has 2 or more available SATA protocol SSD/HDD slots (including MSATA, M.2...
|
by: marktang |
last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
|
by: Hystou |
last post by:
Most computers default to English, but sometimes we require a different language, especially when relocating. Forgot to request a specific language before your computer shipped? No problem! You can...
|
by: Oralloy |
last post by:
Hello folks,
I am unable to find appropriate documentation on the type promotion of bit-fields when using the generalised comparison operator "<=>".
The problem is that using the GNU compilers,...
|
by: jinu1996 |
last post by:
In today's digital age, having a compelling online presence is paramount for businesses aiming to thrive in a competitive landscape. At the heart of this digital strategy lies an intricately woven...
| |
