By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
446,305 Members | 1,619 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 446,305 IT Pros & Developers. It's quick & easy.

2D array of structures

P: n/a
Hello,

I wonder how to resize such array of structures using realloc()?

#include <stdio.h>
#include <stdlib.h>
#define FIRST 7

typedef struct {
char *name;
int i;
int j;
} STRUCTURE;

STRUCTURE **p_structure;

int main() {

p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));
if ( p_structure == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
}

Thank you in advance

Svata

Nov 7 '06 #1
Share this Question
Share on Google+
44 Replies


P: n/a
svata <sv******@centrum.czwrote:
I wonder how to resize such array of structures using realloc()?
#include <stdio.h>
#include <stdlib.h>
#define FIRST 7
typedef struct {
char *name;
int i;
int j;
} STRUCTURE;
STRUCTURE **p_structure;
int main() {
Make that

int main( void )

since your main() takes no arguments.
p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));
Don't cast the return value of malloc(), it only hides your mistake
should you have forgotten to include <stdlib.h>. But, more important,
you allocate here memory for 7 (FIRST) such structures. malloc() returns
a pointer to the start of this memory, which is of type 'STRUCTURE *'
(malloc() actually returns a void pointer but "STRUCTURE *' is the
correct type of a pointer to that memory). But you assign it instead
to a pointer that has type 'STRUCTURE **'. That, combined with your
use of the words "2D array" in the subjct line, leads to the suspicion
that you actually don't want to allocate memory for something that has
similar properties as an array of stuctures, but something more compli-
cated and similar to a 2D array of such structures. And that's what you
definitely won't get with that allocation, whatever the type of the
pointer you assign the return value of malloc() to. So, instead of
starting to guess what you might have intended I think it's better to
ask you to specify a bit more clearly what you intend to do here: do
you just want memory for a simple set of structures or do you want
something like a 2D array of such structures. In the first case you
could "repair" your program by simply defining 'p_structure' as

STTRUCTURE *p_structure;

Perhaps you thenn also might want to replace the line for the allo-
cation by

p_structure = malloc(FIRST * sizeof *p_structure);

because that way you don't have to change that line anymore if you
should decide to change the type of 'p_structure' sometime later.

You also write something about realloc() but I can't see any
use or mentioning of realloc() in the code you posted. But, of
course, you can use realloc() to resize the amount of memory
you obtained from malloc() - that's what realloc() was invented
for.
if ( p_structure == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
It's good to see that you check that what malloc() returned!
}
Since main() returns an int, here's a missing line with a return
value...
Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Nov 7 '06 #2

P: n/a
Jens Thoms Toerring wrote:
svata <sv******@centrum.czwrote:
<snip>
>p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));
<snip>
correct type of a pointer to that memory). But you assign it instead
to a pointer that has type 'STRUCTURE **'. That, combined with your
use of the words "2D array" in the subjct line, leads to the suspicion
that you actually don't want to allocate memory for something that has
similar properties as an array of stuctures, but something more compli-
cated and similar to a 2D array of such structures. And that's what you
<snip>

I agree with what you've said, but the OP should also read section 6 of
the comp.lang.c FAQ before posting back here. Section 6 includes ways of
dynamically allocating space for things that work like 2D arrays. The
FAQ can be found at http://c-faq.com/
--
Flash Gordon
Nov 7 '06 #3

P: n/a
svata:
STRUCTURE **p_structure;

This is a pointer to a pointer to a STRUCTURE.

(I'm against the use of ALL CAPS for anything other than macros)

int main() {

p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));

This doesn't make sense.

You have a pointer to a pointer to a STRUCTURE. This would suggest to me
that you're going to store either of the following in it:

(1) The address of a pointer to a STRUCTURE.
(2) The address of the first element of an array, whereby each element
is a pointer to a STRUCTURE.

If it were either of the two above, the "malloc" invocation should look
something like:

p_structure = malloc( sizeof(STRUCTURE*) );
or:
p_structure = malloc( 12 * sizeof(STRUCTURE*) );

Note that I use the size of a "STRUCTURE*" rather than the size of a
"STRUCTURE".

Of course, it's better to write the two of them as:

p_structure = malloc(sizeof*p);

p_structure = malloc(12 * sizeof*p);

--

Frederick Gotham
Nov 7 '06 #4

P: n/a
svata wrote:
>
I wonder how to resize such array of structures using realloc()?

#include <stdio.h>
#include <stdlib.h>
#define FIRST 7

typedef struct {
char *name;
int i;
int j;
} STRUCTURE;

STRUCTURE **p_structure;
Too many *s here.
>
int main() {
You
>
p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));
and here. Also _never_ cast the return from malloc (it only hides
errors without fixing them). You want:

p_structure = malloc(FIRST * sizeof *p_structure);
if ( p_structure == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
}
You can then change the size to, say, SECOND with:

STRUCTURE *tmp; /* in a suitable location, not here */

if (tmp = realloc(p_structure, SECOND) p_structure = tmp;
else {
/* handle lack of memory */
}

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

Nov 7 '06 #5

P: n/a
Hello Frederick,

I know it doesn't make sense. I was wrong in my assumption.
I should use p_structure = (STRUCTURE **) malloc(FIRST *
sizeof(STRUCTURE*));

but anyway... should rather use array of structures.
typedef structure {
code goes here...
} _structure;

_structure *p_structure;
// and then malloc()

p_structure = malloc( INT * sizeof(_structure));

if am I right?

svata

Frederick Gotham wrote:
STRUCTURE **p_structure;


This is a pointer to a pointer to a STRUCTURE.

(I'm against the use of ALL CAPS for anything other than macros)

int main() {

p_structure = (STRUCTURE **) malloc(FIRST * sizeof(STRUCTURE));


This doesn't make sense.
--

Frederick Gotham
Nov 9 '06 #6

P: n/a
svata wrote:
Hello Frederick,

I know it doesn't make sense. I was wrong in my assumption.
I should use p_structure = (STRUCTURE **) malloc(FIRST *
sizeof(STRUCTURE*));
No, you shouldn't: it's wiser to drop the unnecessary cast an
to ensure that the thing you sizeof is appropriate for the
pointer:

pStructure = malloc( FIRST * sizeof (*pStructure) );

which will (try to) mallocate space for FIRST (horrid name)
lumps of space adequate for the kinds of thing that
pStructure points to.
but anyway... should rather use array of structures.
Yes, get rid of the unnecessary layer of indirection.
typedef structure {
code goes here...
} _structure;
There are two things wrong with this. One is that it's
syntax is broken, since you spelt "struct" "structure".
The other is that so many names beginning with _ are
reserved to the implementation it's unwise for you to
define /any/ name that does. So don't.

If you /must/ use a typedef for a structure -- you don't
need to, and some wise people argue that you shouldn't
(although others argue that those arguments aren't
convincing) -- you should also give it a /sensible/
name. "structure" isn't.

typedef struct yourStructTagHere
{
int x;
int y;
} Point;

--
Chris ".enable proofreading" Dollin
The "good old days" used to be much better.

Nov 9 '06 #7

P: n/a
Chris Dollin wrote:

I learn by doing. So I will see what results I get.
pStructure = malloc( FIRST * sizeof (*pStructure) );

which will (try to) mallocate space for FIRST (horrid name)
lumps of space adequate for the kinds of thing that
pStructure points to.
but anyway... should rather use array of structures.

Yes, get rid of the unnecessary layer of indirection.
typedef structure {
code goes here...
} _structure;

There are two things wrong with this. One is that it's
syntax is broken, since you spelt "struct" "structure".
The other is that so many names beginning with _ are
reserved to the implementation it's unwise for you to
define /any/ name that does. So don't.

If you /must/ use a typedef for a structure -- you don't
need to, and some wise people argue that you shouldn't
(although others argue that those arguments aren't
convincing) -- you should also give it a /sensible/
name. "structure" isn't.

typedef struct yourStructTagHere
{
int x;
int y;
} Point;

--
Chris ".enable proofreading" Dollin
The "good old days" used to be much better.
Nov 9 '06 #8

P: n/a
I always use google to search an answer, but often results are not
relevant.
svata
>
I agree with what you've said, but the OP should also read section 6 of
the comp.lang.c FAQ before posting back here. Section 6 includes ways of
dynamically allocating space for things that work like 2D arrays. The
FAQ can be found at http://c-faq.com/
--
Flash Gordon
Nov 9 '06 #9

P: n/a
svata said:
Chris Dollin wrote:

I learn by doing. So I will see what results I get.
That's fine sometimes, but there are also times when it's best to learn from
other people's knowledge and experience. If you learn C "by doing", you're
likely to end up doing lots of things that aren't correct, but which happen
to behave in a particular way on your current system. Switch systems, and
all your code breaks. Oops.

Chris is an expert on C. Listen to Chris.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 9 '06 #10

P: n/a
svata wrote:
>
I always use google to search an answer, but often results are not
relevant.
>I agree with what you've said, but the OP should also read section
6 of the comp.lang.c FAQ before posting back here. Section 6
includes ways of dynamically allocating space for things that work
like 2D arrays. The FAQ can be found at http://c-faq.com/
I'm sure you've been told this before, but DON'T TOP-POST. See the
links in my sig below.

--
Some informative links:
<news:news.announce.newusers
<http://www.geocities.com/nnqweb/>
<http://www.catb.org/~esr/faqs/smart-questions.html>
<http://www.caliburn.nl/topposting.html>
<http://www.netmeister.org/news/learn2quote.html>
<http://cfaj.freeshell.org/google/>
Nov 9 '06 #11

P: n/a
Richard Heathfield wrote:
svata said:
>Chris Dollin wrote:

I learn by doing. So I will see what results I get.
Something happend to your quoting, Richard: it looks like it was me that
said "I learn by doing. So I will see what results I get.", but it was
svata.

(svata's quoting was misleading anyway, since he half-top-posted.)
That's fine sometimes, but there are also times when it's best to learn from
other people's knowledge and experience. If you learn C "by doing", you're
likely to end up doing lots of things that aren't correct, but which happen
to behave in a particular way on your current system. Switch systems, and
all your code breaks. Oops.

Chris is an expert on C.
I'd hesitate to describe myself so, since there's so much C I don't
know (most of the C99 stuff, for example!) and I've used it in so
few environments. I know /some/ stuff about C. I hope it's useful
stuff.
Listen to Chris.
But not when I'm singing.

--
Chris "for that, you want Annie, Rachel, Anne-Marie, or Tina." Dollin
"Reaching out for mirrors hidden in the web." - Renaissance, /Running Hard/

Nov 9 '06 #12

P: n/a
Chris Dollin wrote:
Richard Heathfield wrote:
.... snip ...
>
>Listen to Chris.

But not when I'm singing.
Like me, you appear to be a member of the 'crows in heat' chorus.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>
Nov 9 '06 #13

P: n/a
CBFalconer wrote:
Chris Dollin wrote:
>Richard Heathfield wrote:
... snip ...
>>
>>Listen to Chris.

But not when I'm singing.

Like me, you appear to be a member of the 'crows in heat' chorus.
Very likely. Or hedgehogs ... snuffling. Perhaps we should
characterise Undefined Behaviour as "Chris or <your preferred
abbreviationwill sing for/at you". /That/ will worry them
more than hyperbolic nasal demons.

--
Chris "we are BRO-KEN, without FEE-LING" Dollin
"Who do you serve, and who do you trust?" /Crusade/

Nov 9 '06 #14

P: n/a
Chris Dollin said:
Richard Heathfield wrote:
>svata said:
>>Chris Dollin wrote:

I learn by doing. So I will see what results I get.

Something happend to your quoting, Richard:
No, just a snip slip. Sorry about that.
>Chris is an expert on C.

I'd hesitate to describe myself so,
Think "relative". Compared to most OPs around here, you're a towering
genius!

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 9 '06 #15

P: n/a
svata:
Hello Frederick,

Hello svata :)

I know it doesn't make sense. I was wrong in my assumption.
I should use p_structure = (STRUCTURE **) malloc(FIRST *
sizeof(STRUCTURE*));

but anyway... should rather use array of structures.
typedef structure {
code goes here...
} _structure;

_structure *p_structure;

One thing that you'll notice as you program more and more is that you'll
come to use abbreviations (most programmers do in anyway). Humans love
shortcuts. "should have" became "should've", which became "shuda". The
words "until" and "because" are slowly but surely becoming "til" and
"cause".

When I myself define a pointer variable, I simply prefix "p" to the name; I
used to put an underscore with it too but then more months passed by and I
got lazier and lazier. The variable name "p_structure" is a bit of a
mouthful, maybe you'd prefer to keep the names small.

There are a few different prevalent coding styles out there. My own style
works as follows:

(1) Functions and Types start with a capital letter. If it consists of more
than one word, than the next word starts with an initial capital, e.g.

void TruncateLastFiveDigits(char*);

(2) Objects start with a lower case letter. If it consists of more than one
word, then underscores are used, e.g.

int len_main_str = 34;

(3) For a pointer, I prefix a simple "p" to the name where possible:

void (*pFunc)(int) = Func;
void (**ppFunc)(int) = &pFunc;

int *pobj = &obj;
int **ppobj = &pobj;
int ***pppobj = &ppobj;

(4) Macro names are in ALL CAPS. No other name should be written in ALL
CAPS, e.g.:

#define LEN 5
#define SQR(x) ((x)*(x))

Of course, this is just my own style. I'm not trying to shove it down your
throat, but I'm just suggesting it as I thought you might like it.

// and then malloc()

p_structure = malloc( INT * sizeof(_structure));

if am I right?

Yes, you're right. It's handy though to not have to repeat the name of the
type:

int *const p = malloc(5 * sizeof(int));

can be written as:

int *const p = malloc(5 * sizeof*p);

Now, if we change the type to double, we only have to change "p":

double *const p = malloc(5 * sizeof*p);

Here's a taste of how I might write the code:

#include <stddef.h /* To use "size_t" */
#include <stdlib.h /* To use "malloc" and "free" */

typedef struct MyStruct {
int i;
} MyStruct;

size_t GetNumberFromSomewhere(void); /* Defined elsewhere */

int main(void)
{
size_t const len = GetNumberFromSomewhere();

MyStruct *const p = malloc(len * sizeof*p);

/* ... */

free(p);

return 0;
}

--

Frederick Gotham
Nov 9 '06 #16

P: n/a
svata wrote:

Please do not top post. Your reply belongs under the text you are
replying to, not above.
svata
>I agree with what you've said, but the OP should also read section 6 of
the comp.lang.c FAQ before posting back here. Section 6 includes ways of
dynamically allocating space for things that work like 2D arrays. The
FAQ can be found at http://c-faq.com/
I always use google to search an answer, but often results are not
relevant.
I did not suggest using Google, I suggested a specific resource and a
particular area in it. From another post here it is obvious you have not
followed that advice, and if you don't bother following advice or
posting properly I see no reason to bother giving you advice.
--
Flash Gordon
Nov 9 '06 #17

P: n/a
Chris Dollin <ch**********@hp.comwrites:
[...]
If you /must/ use a typedef for a structure -- you don't
need to, and some wise people argue that you shouldn't
(although others argue that those arguments aren't
convincing) -- you should also give it a /sensible/
name. "structure" isn't.

typedef struct yourStructTagHere
{
int x;
int y;
} Point;
There's also no need to use different identifiers for the struct tag
and the typedef name:

typedef struct Point {
int x;
int y;
} Point;

Now you can refer to the type either as "Point" or as "struct Point".

The purpose of the typedef is to allow you to use a single identifer
to refer to the type. The argument Chris alluded to above *against*
using a typedef is that gives a second name to a type that already has
a perfectly good name. For example, I would have declared it as:

struct Point {
int x;
int y;
};

and just refer to the type as "struct Point".

But plenty of smart people prefer to use the typedef. And if your
structure doesn't contain any pointers to itself (as for a linked list
node, for example), you don't need the struct tag:

typedef struct {
int x;
int y;
} Point;

The drawback of this is that the name "Point" doesn't become visible
until the end of the declaration, so you can't declare a member of
type Point*.

--
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.
Nov 9 '06 #18

P: n/a
Frederick Gotham wrote:
>
.... snip ...
>
One thing that you'll notice as you program more and more is that
you'll come to use abbreviations (most programmers do in anyway).
Humans love shortcuts. "should have" became "should've", which
became "shuda". The words "until" and "because" are slowly but
surely becoming "til" and "cause".
Not the smarter ones. They try to stick to English, and remain
fairly clear to their readers. Should've is legitimate English,
shuda is an execresence. Misuse of the word 'cause' causes nothing
but confusion.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

Nov 9 '06 #19

P: n/a
Keith Thompson said:
Chris Dollin <ch**********@hp.comwrites:
[...]
>If you /must/ use a typedef for a structure -- you don't
need to, and some wise people argue that you shouldn't
(although others argue that those arguments aren't
convincing) -- you should also give it a /sensible/
name. "structure" isn't.

typedef struct yourStructTagHere
{
int x;
int y;
} Point;

There's also no need to use different identifiers for the struct tag
and the typedef name:

typedef struct Point {
int x;
int y;
} Point;
It's true that there's no C reason to use different identifiers, but I do so
anyway because it helps Microsoft's "Intellisense" to work out what you
mean when you hit the button that says "take me to your definition". Not
that I use Microsoft C terribly often - but when I do, I usually end up
thanking myself for using a unique tag name. My preferred style nowadays
is:

struct foo_
{
char coal;
short wait;
unsigned letter;
long time;
float away;
double trouble;
bar baz;
ad nauseam;
};

typedef struct foo_ foo;

<snip>

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 9 '06 #20

P: n/a
Richard Heathfield <in*****@invalid.invalidwrites:
Keith Thompson said:
[...]
>There's also no need to use different identifiers for the struct tag
and the typedef name:

typedef struct Point {
int x;
int y;
} Point;

It's true that there's no C reason to use different identifiers, but I do so
anyway because it helps Microsoft's "Intellisense" to work out what you
mean when you hit the button that says "take me to your definition". Not
that I use Microsoft C terribly often - but when I do, I usually end up
thanking myself for using a unique tag name.
[...]

Alas, it's sometimes necessary to obfuscate your code to cater to
inferior tools.

--
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.
Nov 10 '06 #21

P: n/a
Keith Thompson said:
Alas, it's sometimes necessary to obfuscate your code to cater to
inferior tools.
I am not convinced that a mere trailing underscore on a tag name constitutes
obfuscation.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #22

P: n/a
Hello,

I meant that I admire people who knows more than I do :) But, I try to
code first, and if I'm stuck I ask for help. I don't expect someone to
do coding for me.
But anyway, thanks for your patience.

svata

Richard Heathfield wrote:
That's fine sometimes, but there are also times when it's best to learn from
other people's knowledge and experience. If you learn C "by doing", you're
likely to end up doing lots of things that aren't correct, but which happen
to behave in a particular way on your current system. Switch systems, and
all your code breaks. Oops.

Chris is an expert on C. Listen to Chris.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #23

P: n/a
I'm sorry for that mess.
(svata's quoting was misleading anyway, since he half-top-posted.)
So, I have to ask. I did my best, my code is almost finished, but I'm
looking for someone who can do code review and comment it. Via email
preferably.
There are some bugs, which I can't get rid of. I use both Borland C
compiler and gcc.
On windows it runs with some drawbacks, on linux it crashes due to bad
use of realloc().

So, is anyone ready for code review please?
Chris is an expert on C.

I'd hesitate to describe myself so, since there's so much C I don't
know (most of the C99 stuff, for example!) and I've used it in so
few environments. I know /some/ stuff about C. I hope it's useful
stuff.
Listen to Chris.
Is your singing so bad? :)
But not when I'm singing.

--
svata

Nov 10 '06 #24

P: n/a
svata said:
I did my best, my code is almost finished, but I'm
looking for someone who can do code review and comment it.
Try posting it, then.
Via email preferably.
Good luck with that, but in general you'll find that the email responders
are those who fear their answers will not survive in the glare of public
scrutiny.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #25

P: n/a
Richard Heathfield wrote:
Chris Dollin said:
>Richard Heathfield wrote:
>>Chris is an expert on C.

I'd hesitate to describe myself so,

Think "relative". Compared to most OPs around here, you're a towering
genius!
You've met me. You know "towering" isn't in my job description.

--
Chris "nor is the G-word ..." Dollin
"People are part of the design. It's dangerous to forget that." /Star Cops/

Nov 10 '06 #26

P: n/a
Richard Heathfield wrote:

The only problem is that the code is supposed as a school task. I do
not fear to reveal my code at all. But would like to avoid my lecturer
to find my code before I submit it.
Good luck with that, but in general you'll find that the email responders
are those who fear their answers will not survive in the glare of public
scrutiny.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #27

P: n/a
Please don't top-post. I have rearranged your post to have what
you were replying to to come first:

svata <sv******@centrum.czwrote:
Richard Heathfield wrote:
Good luck with that, but in general you'll find that the email responders
are those who fear their answers will not survive in the glare of public
scrutiny.
The only problem is that the code is supposed as a school task. I do
not fear to reveal my code at all. But would like to avoid my lecturer
to find my code before I submit it.
First question: why can't you ask you lecturer? Isn't (s)he supposed
to teach you? Or are you supposed to have understood already every-
thing and this is a test if you did?

Next question: are you not allowed to ask others for help? If you're
allowed what's the problem with posting your code here? If you're not
allowed why should we help you cheating? But if you have some other
reason for not posting your code try to write a similar program and,
if it exhibits the same bugs, post that here and apply what you learned
from the replies to the program you have to hand in. As a bonus you may
even figure out yourself what the error is when you try to write a
similar program...
Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Nov 10 '06 #28

P: n/a
Jens Thoms Toerring wrote:
Please don't top-post. I have rearranged your post to have what
you were replying to to come first:
Ok, I remember it from now on :)
>
First question: why can't you ask you lecturer? Isn't (s)he supposed
to teach you? Or are you supposed to have understood already every-
thing and this is a test if you did?
I can, but I'm somehow supposed do my own research.
>
Next question: are you not allowed to ask others for help? If you're
allowed what's the problem with posting your code here? If you're not
allowed why should we help you cheating? But if you have some other
reason for not posting your code try to write a similar program and,
if it exhibits the same bugs, post that here and apply what you learned
from the replies to the program you have to hand in. As a bonus you may
even figure out yourself what the error is when you try to write a
similar program...
I don't ask anyone her to help me to cheat. I can ask for help another
people, of course.
I rather wonder if someone can comment it to help me to learn good
programming habits.

So my code follows:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define FIRST 7
#define MAX 10

// First of variables have to be declared
char answer;
char item_name[MAX];
char *p_item_name;
char string;
int i;
int j;
int str_len;
float amount;
int NEW_SIZE;
typedef struct {
// to store strig entered by user
char *name;
// to be able determine whether it is predefined menu or not, used
when receipt is printed out
int menu;
// to be able determine to which menu group
int group;
// to store price entered by user
float price;
// important as we need to see which index is already used
int used;
} SHOPPING;

SHOPPING *p_shopping;

// Declaring of menu items, necessary for later reuse in array
char *p_menu_items[] = {"Bread", "Butter", "Confectionary", "Fruit",
"Meat", "Milk", "Vegetables", "Other"};

// Declaring choices menu, not being used in any other array
char *p_menu_choices[] = {"Sub-total", "Total", "Quit"};
int main() {

int store_data(int); // declaring we want to use function
// now, allocating memory for first 8 arrays, menu is going to be
stored there
p_shopping = malloc(FIRST * sizeof *p_shopping);
if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}

while (1) { // infinitive loop
// This cannot be part of a menu array, to be printed once only!!
printf("\nEnter your choice: \n");
// Putting menu together by looping through 2 arrays
for ( i = 0; i < (FIRST + 1); i++ ) {
// it's time to allocate memory on the fly, as we have
different string size
p_shopping[i].name = malloc((strlen(p_menu_items[i]) + 1) *
sizeof(char));
if ( p_shopping[i].name == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
//copying content of menu items to shopping structure for later use
strcpy(p_shopping[i].name, p_menu_items[i])
// and set int used to 1, to determine which indexes are used
p_shopping[i].used = 1;
printf("\t\t\t %d. %s\n", (i + 1), p_shopping[i].name );
// as this is predefined menu, we have to state it here, for future
use
p_shopping[i].menu = 1;
// set int group to its value according group of items, for future
use
p_shopping[i].group = i;
}
// second array for menu
for ( i = 0; i < 3; i++ ) {
// Neccessary to align, because there are numbers greater than 9 :)
if ( i < 1 ) {
printf("\t\t\t %d. %s\n", ( i + 9 ), p_menu_choices[i]);
}
else {
printf("\t\t\t%d. %s\n", ( i + 9 ), p_menu_choices[i]);
}
}
// This cannot be a part of menu array, to be printed once only!!
printf("\t\t\t=====");
scanf("%d", &choice);
getchar();
switch(choice) {
// Optional exit, user has to confirm it...
case 0 :
while (1) {
printf("Do you really want to exit? [y/n]: ");
getchar();
scanf("%c", &answer);
getchar();

switch (answer) {
case 'y':
printf("Ok, as you wish, quiting...\n");
return 0;

case 'n':
printf("Ok, returning back...\n");
// had to use go to, otherwise it is almost impossible to escape
infinite loop
goto exit;

default:
printf("Please answer 'y' or 'n'!\n");
break;
}
}

case 1 :
store_data(0);
break;

case 2 :
store_data(1);
break;

case 3 :
store_data(2);
break;

case 4 :
store_data(3);
break;

case 5 :
store_data(4);
break;

case 6 :
store_data(5);
break;

case 7 :
store_data(6);
break;

case 8 :
store_data(7);
break;

case 9 :
store_data(-1);
break;

case 10 :
store_data(-1);
printf("Enter amount tendered: ");
scanf("%f", &amount);
// to test whether amount tendered is greater than total sum
if ( amount < sub_sum ) {
printf("Tendered amount is lower than total sum!!!");
// if so, exit here and user has to enter new choice
break;
}
else {
printf("Your change is %4.2f\n\n", (amount - sub_sum));
}

case 11 :
// user wants to leave
printf("Thank you for shopping\n");
return 1;

default :
// user entered invalid choice
printf("Invalid choice entered, quiting....\n");
return 0;
}
// here points goto from switch 0
exit:
// we do nothing here, just leaving inner loop
;
}
}

int store_data(int group) {
int choice;
float sum;
float sub_sum;
static int SIZE = 0; // need to use persistent variable, which counts
invocation of function
if ( group == -1 ) {
// this is really important, otherwise it screws up the result :)
sub_sum = 0;
// now, receipt is expected to be printed
printf("#########################\n#\t YOUR
RECEIPT\t#\n#########################\n\n");
// as we have 8 main menu items, we want to print them, one by one
for ( j = 0; j < (FIRST + 1); j++ ) { // can use FIRST as we know
that first 8 items are menus...
printf("%s\n-------------------------------\n", p_shopping[j].name);
for ( i = 0; i < (NEW_SIZE + 1); i++ ) { // and now print relevant
submenu
if ( p_shopping[i].group == j && p_shopping[i].menu != 1 ) {
sub_sum += p_shopping[i].price;
printf("%s\t\t\t%4.2f\n", p_shopping[i].name,
p_shopping[i].price);
}
}
// submenu separator :)
printf("\n");
}
printf("------------------------------\n");
printf("Total\t\t\t%4.2f\n\n", sub_sum);
return 0;
}
else {
SIZE ++; // we want keep trace how many times is this part invoked
NEW_SIZE = SIZE + FIRST;
for ( i = 0; i < (NEW_SIZE); i++ ) {
printf("%d\t%d.%d %s\n", i, p_shopping[i].used, p_shopping[i].group,
p_shopping[i].name );
}
printf("Enter description (max. size %d): ", sizeof(item_name));
if (fgets(item_name, sizeof(item_name), stdin) != NULL){
if (( p_item_name = strchr(item_name, '\n')) != NULL )
*p_item_name = '\0';
}
p_shopping[NEW_SIZE].used = 0; // must be set to 0 as it can have
random value
printf("Price: ");
scanf("%f", &sum);
// it's time to resize array now, as we have data we need
p_shopping = realloc(p_shopping, (sizeof(*p_shopping) * (NEW_SIZE +
1)));
if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
}
else {
for ( i = 0; i < (NEW_SIZE + 1); i++ ) {
if ( i == (NEW_SIZE)) {
p_shopping[i].name = malloc((strlen(item_name) + 1) *
sizeof(char));
strcpy(p_shopping[i].name, item_name);
p_shopping[i].used = 1;
p_shopping[i].price = sum;
p_shopping[i].group = group;
}
//printf("%d\t%d.%d %s\n", i, p_shopping[i].used,
p_shopping[i].group, p_shopping[i].name );
}
}
return 0;
}
}

This example has cca 250 lines, I hope it is appropriate to post here.
There is a bug in use of realloc() and so far I was unable to figure it
out. I did a lot of "debuging" by using many printf() commands at
different stages.
So please feel free to comment coding style, semantic etc. I know, this
is not the best solution, but this my attempt to find it. I'm supposed
to learn by doing.

Svata

Nov 10 '06 #29

P: n/a
svata wrote:
Richard Heathfield wrote:

The only problem is that the code is supposed as a school task. I
do not fear to reveal my code at all. But would like to avoid my
lecturer to find my code before I submit it.
>Good luck with that, but in general you'll find that the email
responders are those who fear their answers will not survive in
the glare of public scrutiny.
You have been asked multiple times to stop top-posting, yet you
insist on so doing. I conclude you really do not care whether or
not anyone reads your posts. I shall not be doing so.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>
Nov 10 '06 #30

P: n/a
svata wrote:

// First of variables have to be declared
char answer;
char item_name[MAX];
char *p_item_name;
char string;
int i;
int j;
int str_len;
float amount;
int NEW_SIZE;
That's rather a lot of globals for a problem this size.
Most of those look like candidates for locals. If they're
going to be globals, they should have better names.
int main() {

int store_data(int); // declaring we want to use function
Usually put /outside/ functions, not inside.

For heaven's sake, /split this code up/. Functions are /useful/ and
one of C's few structuring tools.

>
case 1 :
store_data(0);
break;

case 2 :
store_data(1);
break;

case 3 :
store_data(2);
break;

case 4 :
store_data(3);
break;

case 5 :
store_data(4);
break;

case 6 :
store_data(5);
break;

case 7 :
store_data(6);
break;

case 8 :
store_data(7);
break;
Duplication like this should make your blood run cold and your
hair stand on end, as if you were standing next to a Van der
Graff generator and the arcing were imminent. At least you
can write:

case 1: case 2: ... case 8: store_data( choice - 1 ); break;

(Layout to taste, I just wanted one line for the post.)

--
Chris ".enable proofreading" Dollin
"Life is full of mysteries. Consider this one of them." Sinclair, /Babylon 5/

Nov 10 '06 #31

P: n/a
svata wrote:

I rather wonder if someone can comment it to help me to learn good
programming habits.

So my code follows:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define FIRST 7
#define MAX 10

// First of variables have to be declared
Right away, this raft of global variables raises many flags. There are
occasions when globals are the right solution, but for the most part
it's a sign of a lazy or inexperienced programmer.
char answer;
char item_name[MAX];
char *p_item_name;
char string;
"string" is declared as a single character. The name doesn't fit the
type, and is probably an error. I imagine you wanted a character buffer.
int i;
int j;
int str_len;
float amount;
int NEW_SIZE;
Try to reserve all caps for macros.
>
typedef struct {
// to store strig entered by user
// comments are a feature of c99, although often an available
extension. We try to avoid them here due to line-wrapping.
char *name;
// to be able determine whether it is predefined menu or not, used
when receipt is printed out
int menu;
// to be able determine to which menu group
int group;
// to store price entered by user
float price;
// important as we need to see which index is already used
int used;
} SHOPPING;
Again, don't use all-caps.
SHOPPING *p_shopping;

// Declaring of menu items, necessary for later reuse in array
char *p_menu_items[] = {"Bread", "Butter", "Confectionary", "Fruit",
"Meat", "Milk", "Vegetables", "Other"};
This should be:

const char *p[] = . . .

String literals are not modifiable.

// Declaring choices menu, not being used in any other array
char *p_menu_choices[] = {"Sub-total", "Total", "Quit"};
Same here.
>
int main() {

int store_data(int); // declaring we want to use function
// now, allocating memory for first 8 arrays, menu is going to be
stored there
p_shopping = malloc(FIRST * sizeof *p_shopping);
FIRST is 7, yet you say you are allocating eight. Which is it? Also,
why dynamic allocation
if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;
Use the standard return values: 0, EXIT_SUCCESS, or EXIT_FAILURE.
}

while (1) { // infinitive loop
// This cannot be part of a menu array, to be printed once only!!
printf("\nEnter your choice: \n");
You prompt for a choice, but I don't see any input statement.
// Putting menu together by looping through 2 arrays
for ( i = 0; i < (FIRST + 1); i++ ) {
// it's time to allocate memory on the fly, as we have
different string size
p_shopping[i].name = malloc((strlen(p_menu_items[i]) + 1) *
sizeof(char));
p_shopping has only 7 structs in the array, with valid indexes of 0-6.
You are attempting to use up to index 7. Boom (if you're lucky).

I'm going to stop here. You have too many problems. You tried to write
the whole program at once, and that seldom works. Even experienced
programmers don't do it that way.

Start over. Start small. Build up the pieces. Get your menu selection
working first. Use functions. At the early stages, those will be stubs,
then get their code expanded.

Brian
Nov 10 '06 #32

P: n/a
svata said:
Jens Thoms Toerring wrote:
>>
First question: why can't you ask you lecturer? Isn't (s)he supposed
to teach you? Or are you supposed to have understood already every-
thing and this is a test if you did?

I can, but I'm somehow supposed do my own research.
Consulting experts doesn't count as research?
I don't ask anyone her to help me to cheat.
None of the experts here will help you to cheat, but we will help you to
learn, if you're willing for that to happen.
I can ask for help another people, of course.
That's up to you, of course.
I rather wonder if someone can comment it to help me to learn good
programming habits.
Sure.
So my code follows:
Fabulous.

Okay, here's the result of my first compilation:

foo.c:59: unterminated character constant
foo.c:198: warning: string constant runs past end of line
foo.c:235: unterminated character constant
make: *** [foo.o] Error 1

So let's take a look at those. Here's the first:

// it's time to allocate memory on the fly, as we have
different string size

This is the preprocessor complaining about the single, unmatched apostrophe.
Yes, it's in a comment, but it's a new-style C99 comment which not all
implementations understand (and in fact implementations are obliged to
diagnose it as a syntax error unless, of course, they are C99 compilers,
which are rather rare).

As you can see from the quote, the comment also got line-wrapped by Usenet,
making it spill into a second line, which can only cause trouble when the
preprocessor finally hands over to the compiler.

My fix for this was to remove all comment lines completely.

Here are line 198 and 199:

printf("#########################\n#\t YOUR
RECEIPT\t#\n#########################\n\n");

My fix for this is as follows:

printf("#########################\n#\t YOUR "
"RECEIPT\t#\n#########################\n\n");

My next step was to re-format the program to render it more readable, and in
any case I've removed all the // comments, so the line numbers won't match
yours from now on.

foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",

I fixed this by using const char *, and I couldn't resist fixing the
spelling mistake at the same time:

const char *p_menu_items[] =
{
"Bread", "Butter", "Confectionery", "Fruit",
"Meat", "Milk", "Vegetables", "Other"
};

const char *p_menu_choices[] =
{
"Sub-total", "Total", "Quit"
};

Next was this:

foo.c:38: warning: function declaration isn't a prototype

I fixed this by changing:

int main()

to

int main(void)

foo.c:44: warning: nested extern declaration of `store_data'

I fixed this by shifting the declaration to file scope. Types at file scope,
objects at function scope. Types at file scope, objects at function scope.
Types at file scope, objects at function scope. Types at file scope,
objects at function scope. Types at file scope, objects at function scope.
Write it out ten times if need be - or a hundred times.

foo.c:71: parse error before `p_shopping'

strcpy(p_shopping[i].name, p_menu_items[i])
p_shopping[i].used = 1;

I checked your original code, and the same mistake is clearly there too - a
missing semicolon at the end of the strcpy call. No matter how lax your
compiler diagnostic settings, this is a show-stopping error.

You spoke of good programming habits. I would suggest the following list to
begin with:

1) use function scope for objects, not file scope;
2) avoid scanf until you're an expert; if your teacher requires its use,
plague him mercilessly with questions about it until it becomes apparent
that he doesn't understand it either;
3) if you must use // comments, make sure they're sufficiently short that
they are not broken into two or more lines when you post to Usenet;
4) use shorter functions - your main(), even shorn of comments, is around
140 lines long. Short, simple functions work best.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #33

P: n/a
Richard Heathfield <in*****@invalid.invalidwrites:
[...]
foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",
[...]

A technical point: This particular warning message is a bit
misleading. A warning is certainly appropriate here, but it should be
more specific.

The underlying problem is that string literals are not "const", but
attempting to modify a string literal invokes undefined behavior. gcc
has an option, "-Wwrite-strings", that causes it to warn about
attempts to modify string literals. But it does so by *pretending*
that string literals really are const. In this case, it's warning
that the "const" qualifier for the string literals is being discarded,
but in fact there is no such qualifier, either explicitly or
implicitly.

A compiler can warn about anything it likes, and the standard doesn't
require warnings to make sense, so this isn't a conformance issue.
And if you modify the code until all warnings disappear (which is
often a very good idea), this particular warning will have served its
purpose.

--
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.
Nov 10 '06 #34

P: n/a
Keith Thompson said:
Richard Heathfield <in*****@invalid.invalidwrites:
[...]
>foo.c:30: warning: initialization discards qualifiers from pointer target
type

Lots of these warnings, actually, and they are caused by:

char *p_menu_items[] = { "Bread",
[...]

A technical point: This particular warning message is a bit
misleading. A warning is certainly appropriate here, but it should be
more specific.
Yes, some of gcc's diagnostics leave much to be desired. Nevertheless, as
you rightly point out (in another part of your reply), this warning does
serve a purpose, however ineptly.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 10 '06 #35

P: n/a
svata <sv******@centrum.czwrote:
I don't ask anyone her to help me to cheat.
That's good to know;-) And my question wasn't meant as an insult but
just to remind you that asking her for help if you're not allowed to
is a stupid idea (one reason of many being that your lecturere could
very well be reading this group;-)
So my code follows:
<program code snipped>

Sorry, but as others already have told you, your program doesn't even
compile. In main() neither 'choice' nor 'sub_sum' are defined. It's
simple to add 'choice' but the value of 'sub_sum' gets used but never
set - obviously, you would you like that to be the value of the variable
with the same name defined in store_data(), but that's only visible
in that function. not in main().

And your program is messing around with memory in bad ways. To start
with, you allocate memory for one menu item less than you have (FIRST
is 7, but you have 8 menu items) and treat the memory you got as if
there would be enough. Then, in the infinite loop in main(), you
allocate memory for the menu strings each time round. This is, of
course, useless and you also lose the pointers to memory you already
got before, so you're unable to deallocate it - you're program is
"leaking" memory. Things only get worse in your function store_data(),
there are lots of off-by-one errors. No wonder you earlier or later
get a segmentation fault, it's all rather a mess.

I think that you should rewrite that program. And the rewrite should
include a complete redesign of the data structures. At the moment you
store "generic" goods like "Fruit" in the same kind of structure you
use for real goods, e.g. "Apples" or "Bananas". This doesn't make much
sense and makes it hard to understand your program (I am still not sure
what it's actually supposed to do...) as well as dealing with the
different kinds of things. Picking good data structures is essential
for writing good programs. Here's one way you could do it, using
different types of structures for the categories (basically equivalent
to your main menus entries) and the "real" goods the user buys:

typdef structure {
const char *name; /* name of "real" item bought */
double price; /* it's price */
} Goods_T;

typedef structure {
const char *name; /* name of category i.e. "Fruit" */
Goods_T *list; /* of bought goods belonging to this category */
int list_length; /* length of the array of bought goods */
double sub_total; /* total price of all goods in this category */
} Category_T;

You would start with allocating memory for as many category structures
as needed (8 at the moment). For each category you assign a name from the
p_menu_items array (no memory allocation needed - you already got memory
for these strings when you defined p_menu_items, so you just need to
assign a pointer to the relevant element of that array to the name member),
and you set 'list' to NULL, 'list_len' to 0 and 'total' to 0.0. Of course,
you can also start with simply defining an array of these structures and
initialize them in a single step, so you don't need malloc() at all for
the categories:

Category_T categories [ ] = { { "Bread", NULL, 0, 0.0 },
{ "Butter", NULL, 0, 0.0 },
... };

In that case you also don't need the p_mebu_items array anymore.

This out of the way you can ask the user what (s)he wants to buy. When
(s)he selects one of the menu items and then enters the name for some
"real" item you increase the length of the list of bought goods simply
by using realloc() on the 'list' member of the category structure the
item is supposed to go in - if you didn't know, you can use realloc()
also when you don't have any memory yet, if you call it with NULL as
the first argument it behaves like malloc(). Don't forget to increment
the list length variable. For the new list element you now allocate
memory for the name and the you set the price. You also add price to
the sub_total member of the category structure - that way you can keep
a sum for each of the categories, and when you have to print out the
final total you have to iterate over just the categories.

I guess that when you use data structures more suited to your problem
also writing the program will become simpler - you can have functions
that deal with categories and functions for individual items the user
buys and you don't have to do lots of checks what you're dealing with
at any moment. That, in turn, will make your program easier to under-
stand and thus to debug if there still are bugs.

With the new data structures spend some time writing functions. You
should have a function to add a new item to one of the categories, you
should have a function to print a category, using another function that
prints a single item etc. Have functions for the different types o
user input instead of mixing it all together. In the end, you will
have a very simple main() function that contains a loop and just a few
function calls. And each function will also be simple and easy to test.

Regards, Jens

PS:
// Neccessary to align, because there are numbers greater than 9 :)
if ( i < 1 ) {
printf("\t\t\t %d. %s\n", ( i + 9 ), p_menu_choices[i]);
}
else {
printf("\t\t\t%d. %s\n", ( i + 9 ), p_menu_choices[i]);
}

can be replaced by

printf("\t\t\t%2d. %s\n", ( i + 9 ), p_menu_choices[i]);

"%2d" means: use at least two places to print that int, if necessary
putting a space char in front.
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de
Nov 11 '06 #36

P: n/a
Richard Heathfield wrote:
Consulting experts doesn't count as research?
Hopefully yes.
None of the experts here will help you to cheat, but we will help you to
learn, if you're willing for that to happen.
Yes, I'm willing for that.
You spoke of good programming habits. I would suggest the following list to
begin with:

1) use function scope for objects, not file scope;
I will do my best to try it out.
2) avoid scanf until you're an expert; if your teacher requires its use,
plague him mercilessly with questions about it until it becomes apparent
that he doesn't understand it either;
Can you explain me why should I refrain from using scanf()? Is that
function cursed or what? I see real difference in using gets() of
fgets(). But Borland C compiler gives no warning about using gets...

So, please, what I am supposed to use instead of scanf()?
3) if you must use // comments, make sure they're sufficiently short that
they are not broken into two or more lines when you post to Usenet;
Yes, this is a mistake... I will keep eye on that
4) use shorter functions - your main(), even shorn of comments, is around
140 lines long. Short, simple functions work best.
I know, but as everyone can imagine here, it distincts a good/skilled
programmer from a newbie. So I'm the newbie and I have to get used to
it.

But anyway, many thanks for your time.

svata

Nov 11 '06 #37

P: n/a
CBFalconer wrote:
You have been asked multiple times to stop top-posting, yet you
insist on so doing. I conclude you really do not care whether or
not anyone reads your posts. I shall not be doing so.
Sorry, only Jens asked me so far... and since that time I don't do it.

svata

Nov 11 '06 #38

P: n/a
svata said:

<snip>
Can you explain me why should I refrain from using scanf()? Is that
function cursed or what?
No, but it's very easy to misuse it. Most teachers teach it badly. Yours
does, for example.
I see real difference in using gets() of fgets().
Good.
But Borland C compiler gives no warning about using gets...
It isn't required. It's just a good idea.
So, please, what I am supposed to use instead of scanf()?
In my experience, the best way to capture input is as a string. Then parse
it yourself if you were hoping for numbers and stuff.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 11 '06 #39

P: n/a
svata wrote:
Richard Heathfield wrote:
<snip>
>2) avoid scanf until you're an expert; if your teacher requires its use,
plague him mercilessly with questions about it until it becomes apparent
that he doesn't understand it either;

Can you explain me why should I refrain from using scanf()? Is that
function cursed or what? I see real difference in using gets() of
fgets(). But Borland C compiler gives no warning about using gets...
OK, you are paying attention to warnings. That is a good thing. NEVER
use gets. Never. Not under any circumstances. The way gets works is that
you pass it a pointer to a buffer but it has absolutely no knowledge of
how long the buffer is. So you pass a pointer to a buffer 10 characters
long and the user enters 10 character and gets writes off the end of the
buffer stomping over some random piece of data. Well, in general the
piece of data stomped on is not random, and as a result of the I believe
people have in the past successfully exploited buffer overflows where
gets was used to do nasty things.
So, please, what I am supposed to use instead of scanf()?
fgets, fgetc or getc are generally good starting points. fgets and getc
both have gotchas that you have to be wary of but once you know them
they are easy to use correctly.

getc is a macro that is allowed to evaluate its argument more than once,
so doing getc(file_array[i++]) you would not know how many times I would
be incremented.

fgets leave the trailing newline character on the end of the data *if*
the buffer was large enough, otherwise it only reads in as much data as
will fit leaving room for the /0 to terminate the string, which it
always writes. So you have to check to see if the string has a newline
at the end to know if you read the complete line and if not decide how
to deal with only having read part of a line, one option being using
getc/fgetc to read and discard characters until you hit a new line or
end of file.

fgetc hasneither of these problems.
>3) if you must use // comments, make sure they're sufficiently short that
they are not broken into two or more lines when you post to Usenet;

Yes, this is a mistake... I will keep eye on that
It is better not to use // comments at all for posting to news groups,
then the problem does not arrive and it also gives those with C90
compliant compilers a chance of trying the code without editing it.
>4) use shorter functions - your main(), even shorn of comments, is around
140 lines long. Short, simple functions work best.

I know, but as everyone can imagine here, it distincts a good/skilled
programmer from a newbie. So I'm the newbie and I have to get used to
it.

But anyway, many thanks for your time.
A newbie who is prepared and able to learn does not stay a newbie. So
learn, fix the problems that you can in your code, and post again to see
what else can be learned.
--
Flash Gordon
Nov 11 '06 #40

P: n/a
svata wrote:
Richard Heathfield wrote:
.... snip ...
>
>2) avoid scanf until you're an expert; if your teacher requires
its use, plague him mercilessly with questions about it until it
becomes apparent that he doesn't understand it either;

Can you explain me why should I refrain from using scanf()? Is that
function cursed or what? I see real difference in using gets() of
fgets(). But Borland C compiler gives no warning about using gets...
In reverse order, no warning is required for gets avoidance, just
common sense, because it cannot be used safely. You can use my
ggets instead (see url below).

Yes, scanf is cursed, especially for interactive use. The only
sane way to use it is for one item at a time, carefully checking
its return value. Unless your name is Dan Pop.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

Nov 11 '06 #41

P: n/a
Richard Heathfield wrote:
svata said:

<snip>
>Can you explain me why should I refrain from using scanf()? Is that
function cursed or what?

No, but it's very easy to misuse it. Most teachers teach it badly. Yours
does, for example.
Very much agreed. So why not write a book about it (or at least a chapter)?
Your writing is good reading.

Long time ago I used (f)scanf extensively and it's appropriate if you know
what you do.
NoKo
--
"1 + 1 = 3, for suitable large 1"
Nov 12 '06 #42

P: n/a
Norbert Kolvenbach said:
Richard Heathfield wrote:
>No, but it's very easy to misuse [scanf]. Most teachers teach it badly.
[...]
>
Very much agreed. So why not write a book about it (or at least a
chapter)?
Oh, please don't ask me to do that! :-)

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 12 '06 #43

P: n/a
Richard Heathfield wrote:
Oh, please don't ask me to do that! :-)
But then novices like me will ask same questions again and again. I
admit, scanf is a bit tricky and can cause headache if used improperly.

svata

Nov 13 '06 #44

P: n/a
svata said:
Richard Heathfield wrote:
>Oh, please don't ask me to do that! :-)

But then novices like me will ask same questions again and again.
Point taken. Added to my "to-do" list. Near the bottom somewhere, though.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.
Nov 13 '06 #45

This discussion thread is closed

Replies have been disabled for this discussion.