code snippet


B

Bill Cunningham

I have this code snippet that won't compile with gcc -c and I get errors
of needing casts. This is untested code of course and I thought I'd ask for
opinions on what I've done wrong.

Thanks,

#include <stdio.h>

void *get(void *pv)
{
double price;
long int date;
unsigned int number;
int check;
FILE *fp;
if ((fp = fopen("num", "r")) == NULL) {
perror("fopen error");
return 1;
}
if ((check = fscanf(fp, "%u %f %l", &number, &price, &date)) == EOF) {
printf("%d\n", ferror(fp));
clearerr(fp);
return 1;
}

Bill
 
Ad

Advertisements

G

Geoff

I have this code snippet that won't compile with gcc -c and I get errors
of needing casts. This is untested code of course and I thought I'd ask for
opinions on what I've done wrong.

Thanks,

#include <stdio.h>

void *get(void *pv)
{
double price;
long int date;
unsigned int number;
int check;
FILE *fp;
if ((fp = fopen("num", "r")) == NULL) {
perror("fopen error");
return 1;
}
if ((check = fscanf(fp, "%u %f %l", &number, &price, &date)) == EOF) {
printf("%d\n", ferror(fp));
clearerr(fp);
return 1;
}

Bill

Apart from the missing closing brace?

The compiler is expecting your function to return a pointer. You are
not doing that. Furthermore, not all paths through the function as
shown return a value.
 
B

Bill Cunningham

Geoff said:
Apart from the missing closing brace?
Oh...Yeah.

The compiler is expecting your function to return a pointer. You are
not doing that. Furthermore, not all paths through the function as
shown return a value.

It looks to me that they all return a 1 on error and I guess I need to
fix that. I would need to set a pointer to a value of 1. Let's see...

int *error=1;

And in the functions changed to return error;
Sound good.

I forget with this new bash I have been working with how to redirect to
stderr. Maybe this will undo the errors.

Bill
 
G

Geoff

Furthermore, not all paths through the function as

What do you mean?

Bill

You have two 'if' statements that encompass blocks that return 1,
apparently indicating a failure, you neglect the third path where both
ifs fail to evaluate true and you return nothing.

Your function is using the return value to indicate success or failure
but you fail to handle the success case.

You also set the value of 'check' but don't do anything with it.
 
B

Bill Cunningham

Keith said:
Bill Cunningham said:
Geoff wrote: [...]
The compiler is expecting your function to return a pointer. You are
not doing that. Furthermore, not all paths through the function as
shown return a value.

It looks to me that they all return a 1 on error and I guess I
need to fix that. I would need to set a pointer to a value of 1.
Let's see...

int *error=1;

And in the functions changed to return error;
Sound good.

Ask yourself whether 1 is a meeaningful, or even possible, value of
type int*. Or ask your compiler; try compiling the above line of
code before posting it here.

The compiler doesn't like it that's for sure. See below as to why I
haven't posted the errors. 4.6.1's errors are a little different than I'm
used to.
If you mean that you're going to redirect stderr to /dev/null, sure
that will supress all the warning messages -- like taping over that
annoying red light on your car's dashboard.

No no. I want to copy gcc's errors to a text file so I can post it here.
To stdere and not stdout.
 
Ad

Advertisements

B

Bill Cunningham

Geoff said:
You have two 'if' statements that encompass blocks that return 1,
apparently indicating a failure, you neglect the third path where both
ifs fail to evaluate true and you return nothing.

Your function is using the return value to indicate success or failure
but you fail to handle the success case.

You also set the value of 'check' but don't do anything with it.

I don't quite understand fscanf's returns. The man page is very
complicated and I just assumed in my code that fscanf would return what it
needed to if not EOF for error or end of file. Are you reading from man
fscanf ?

Bill
 
G

Geoff

I don't quite understand fscanf's returns. The man page is very
complicated and I just assumed in my code that fscanf would return what it
needed to if not EOF for error or end of file. Are you reading from man
fscanf ?

I'm not looking at fscanf at all, I'm not even concerned if it's
appropriate for the use you're putting it to. I am looking at the code
you wrote around it. You take the return value from fscanf and put it
in the integer check, then you don't do anything with check and you
don't do anything with number, price or date.

You are writing idiomatic code without understanding the idioms or the
reason for them.

You prototyped 'get' wrong, you pass it a pointer, (pv) that you don't
use, you stuff values into variables that you don't use and you return
int when you should return a pointer.

Consider writing less idiomatically and write straight-forward code:

// blah, blah

check = fscanf(fp, "%u %f %l", &number, &price, &date);
if (check == EOF)
{
printf("%d\n", ferror(fp));
clearerr(fp);
return 1;
}
else
{
// do something with number, price and date
}
return 0;
}
 
B

Bill Cunningham

Geoff said:
I'm not looking at fscanf at all, I'm not even concerned if it's
appropriate for the use you're putting it to. I am looking at the code
you wrote around it. You take the return value from fscanf and put it
in the integer check, then you don't do anything with check and you
don't do anything with number, price or date.

Other functions are going to do things with the data gathered from the
function get. What get returns is supposed to be passed to other functions.

BIll
 
G

Geoff

Other functions are going to do things with the data gathered from the
function get. What get returns is supposed to be passed to other functions.

That's great, except that you have declared number, price and date as
local variables within the function and they will go out of scope the
moment 'get' returns.
 
B

Ben Bacarisse

Bill Cunningham said:
I have this code snippet that won't compile with gcc -c and I get errors
of needing casts. This is untested code of course and I thought I'd ask for
opinions on what I've done wrong.

I'll make a few general remarks...
#include <stdio.h>

void *get(void *pv)
{
double price;
long int date;
unsigned int number;
int check;
FILE *fp;
if ((fp = fopen("num", "r")) == NULL) {
perror("fopen error");
return 1;
}
if ((check = fscanf(fp, "%u %f %l", &number, &price, &date)) == EOF) {

(you need %lf to read a double. %f is for float)
printf("%d\n", ferror(fp));
clearerr(fp);
return 1;
}

This is an example of what I'd call programming upside down. Look at
all the detail there: the file name, the format, the error messages, the
clearing of the IO error and so on, but *no code to do any real work*.
You've started walking, but you have no idea where you are going. A
clue to that is the function prototype. The name is entirely
noncommittal, and while there are valid uses of void *, here I think
they mean "I don't know what I need to pass to this function and I have
no idea what it should return".

When I program, I usually start with an outline of the overall behaviour
i.e. I start with main. This is broken down into functions that can do
the constituent parts of the job. Even these functions are often
written in outline first with the focus on what they actually do. The
parts you've worked out in detail, I will leave for later. For one
thing, I want to start testing as soon as possible, so I need a function
that works right away. If I find, either in testing or when finishing
these sketched functions, that things are going wrong, I can change the
design before I've sunk too much effort into it.

So my functions usually start out having a very specific name and
prototype:

double get_stock_price_on_date(FILE *fp, long int date)

but the body will often be what's called a stub:

double get_price_on_date(FILE *fp, long int date)
{
// Rewind file.
// Loop until third field equals date
// If date not found I'll return a negative price.
return 42;
}

Once it's all written like this I can start testing and filling in
the details as I go. If you can't do this, it is often because you
don't know, in enough detail, what your program is to do. It's almost
impossible to program with out a clear objective, so when I find myself
unable to sketch things out like this, it means I need to think more
about my objectives.

A detail: always test for fscanf success. As you say elsewhere the
returns are complex but if it returns 3 you got the three things you
need and you can do what you need to with them. You can (at least
initially) treat any return that is not 3 as an error. You can waste
ages dealing with all the various ways in which things have gone wrong,
so start instead with writing what happens when the input worked.
 
Ad

Advertisements

R

Rui Maciel

Bill said:
It looks to me that they all return a 1 on error and I guess I need to
fix that. I would need to set a pointer to a value of 1. Let's see...

int *error=1;

And in the functions changed to return error;
Sound good.

I forget with this new bash I have been working with how to redirect to
stderr. Maybe this will undo the errors.

Bill

Do you have the freedom to redefine the get() function? If so, I would
suggest that you defined it so that it returned a meaningful result instead
of a pointer. For example, you could define an enum which encompasses all
possible error codes and then rely on that enum definition to return error
codes which are actually meaningful. By doing so, your code would become a
bit easier to read and understand (i.e., no more magic numbers), not to
mention a bit more structured.

For example:

<code>
enum ErrorCodes
{
ERR_OK = 0,
ERR_FOPEN_ERROR,
//...
ERR_UNKNOWN // only returned in SNAFU cases
}

enum ErrorCodes get(void *pv)
{
double price;
long int date;
unsigned int number;
int check;
FILE *fp;
if ((fp = fopen("num", "r")) == NULL) {
perror("fopen error");
return ERR_FOPEN_ERROR;
}
if ((check = fscanf(fp, "%u %f %l", &number, &price, &date)) == EOF) {
printf("%d\n", ferror(fp));
clearerr(fp);
return ERR_FOPEN_ERROR;
}

//...

return ERR_UNKNOWN; // in case of something going horribly wrong
}


int main(void)
{
// snip

enum ErrorCodes error;
error = get(foo);

if(error != ERR_OK)
{
switch(error) // this could also be a predefined function
{
// handle any potential error in here
}
}

// snip
return 0;
}

</code>


Hope this helps,
Rui Maciel
 
B

Bill Cunningham

Rui said:
Do you have the freedom to redefine the get() function? If so, I
would suggest that you defined it so that it returned a meaningful
result instead of a pointer. For example, you could define an enum
which encompasses all possible error codes and then rely on that enum
definition to return error codes which are actually meaningful. By
doing so, your code would become a bit easier to read and understand
(i.e., no more magic numbers), not to mention a bit more structured.

For example:

<code>
enum ErrorCodes
{
ERR_OK = 0,
ERR_FOPEN_ERROR,
//...
ERR_UNKNOWN // only returned in SNAFU cases
}

enum ErrorCodes get(void *pv)
{
double price;
long int date;
unsigned int number;
int check;
FILE *fp;
if ((fp = fopen("num", "r")) == NULL) {
perror("fopen error");
return ERR_FOPEN_ERROR;
}
if ((check = fscanf(fp, "%u %f %l", &number, &price, &date)) ==
EOF) { printf("%d\n", ferror(fp));
clearerr(fp);
return ERR_FOPEN_ERROR;
}

//...

return ERR_UNKNOWN; // in case of something going horribly wrong
}


int main(void)
{
// snip

enum ErrorCodes error;
error = get(foo);

if(error != ERR_OK)
{
switch(error) // this could also be a predefined function
{
// handle any potential error in here
}
}

// snip
return 0;
}

</code>

I can change the parameters I guess but I want this function to return
at times an unsigned int, sometimes a double, sometimes a signed int.
The only way I know to return multiple types without getting into the
possibility of structs is a generic pointer to point to where the values are
stored.

Bill
 
B

Bill Cunningham

Ben said:
I'll make a few general remarks...


(you need %lf to read a double. %f is for float)


This is an example of what I'd call programming upside down. Look at
all the detail there: the file name, the format, the error messages,
the clearing of the IO error and so on, but *no code to do any real
work*. You've started walking, but you have no idea where you are
going. A clue to that is the function prototype. The name is
entirely noncommittal, and while there are valid uses of void *, here
I think they mean "I don't know what I need to pass to this function
and I have no idea what it should return".

When I program, I usually start with an outline of the overall
behaviour i.e. I start with main. This is broken down into functions
that can do the constituent parts of the job. Even these functions
are often written in outline first with the focus on what they
actually do. The parts you've worked out in detail, I will leave for
later. For one thing, I want to start testing as soon as possible,
so I need a function that works right away. If I find, either in
testing or when finishing these sketched functions, that things are
going wrong, I can change the design before I've sunk too much effort
into it.

So my functions usually start out having a very specific name and
prototype:

double get_stock_price_on_date(FILE *fp, long int date)

but the body will often be what's called a stub:

double get_price_on_date(FILE *fp, long int date)
{
// Rewind file.
// Loop until third field equals date
// If date not found I'll return a negative price.
return 42;
}

The thing is your function always returns a double. I want my get to
return multiple types. The only 2 things I know to do is use a pointer of a
type that points to the location of stock price, or line number, or date.
The only other thing I know to do is pack the various types into one type. A
struct. Does this make sense?
 
B

Bill Cunningham

Geoff said:
That's great, except that you have declared number, price and date as
local variables within the function and they will go out of scope the
moment 'get' returns.

Good point.

Bill
 
M

Morris Keesan

....
....

I want my get to
return multiple types. The only 2 things I know to do is use a pointer
of a type that points to the location of stock price, or line number,
or date. The only other thing I know to do is pack the various types
into one type. A struct. Does this make sense?

Look at this style, where you're returning a (void *) which points to
either an unsigned int, a long int, or a double. When another function
calls this get() function, how will it know what type is being returned?

If I were writing a function like this which could return multiple types,
I would have it return an enum which indicates which of the various types
is being returned. This enum could either be an element of a struct
whose other element is a union which could contain any one of the other
types, or I might pass in pointers to each of those types, and return
an enum type telling which of the pointers was used. I would also
define one or more error values for the enum object, which would
indicate that none of the data objects was populated.
 
Ad

Advertisements

B

Bill Cunningham

Ben said:
I'll make a few general remarks...

Code:
This is an example of what I'd call programming upside down.  Look at
all the detail there: the file name, the format, the error messages,
the clearing of the IO error and so on, but *no code to do any real
work*. You've started walking, but you have no idea where you are
going.  A clue to that is the function prototype.  The name is
entirely noncommittal, and while there are valid uses of void *, here
I think they mean "I don't know what I need to pass to this function
and I have no idea what it should return".

When I program, I usually start with an outline of the overall
behaviour i.e. I start with main.  This is broken down into functions
that can do the constituent parts of the job.  Even these functions
are often written in outline first with the focus on what they
actually do.  The parts you've worked out in detail, I will leave for
later.  For one thing, I want to start testing as soon as possible,
so I need a function that works right away.  If I find, either in
testing or when finishing these sketched functions, that things are
going wrong, I can change the design before I've sunk too much effort
into it.

So my functions usually start out having a very specific name and
prototype:

double get_stock_price_on_date(FILE *fp, long int date)

but the body will often be what's called a stub:[/QUOTE]

What is a stub? Never heard of it.

[snip]
 
J

John Gordon

In said:
What is a stub? Never heard of it.

A stub is an empty function. Its purpose is to serve as a placeholder
for the "real" function which you haven't written yet. It allows you to
design the high-level flow of your program, and have it compile, without
getting bogged down in low-level details.
 
R

Rui Maciel

Bill said:
I can change the parameters I guess but I want this function to return
at times an unsigned int, sometimes a double, sometimes a signed int.
The only way I know to return multiple types without getting into the
possibility of structs is a generic pointer to point to where the values
are stored.

Are you, by any chance, developing a parser? If you are, there are better
ways of handling values extracted from tokens.

But regarding the pointer thing, there are other ways to get that job done.
You can define a union which encompasses every data type which you wish your
function to return, and then rewrite your function so that it returns that
instead of a pointer. In fact, relying on a pointer to do that ends up
being a dirty, convoluted way to do what unions do. Incidentally, the code
generated by GNU Bison happens to rely on unions to handle values extracted
from tokens, and it appears to work well.


Rui Maciel
 
Ad

Advertisements

B

Bill Cunningham

Rui said:
Are you, by any chance, developing a parser?

I don't know. I was going to manually type data into a a text file and
read using fscanf. Maybe a parser is what I want.

If you are, there are
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

code 34
error 28
comparison error 12
no error by fscanf on reading from output file 18
fseek 17
malloc 40
Working with files 1
Cgi and file access 8

Top