Function parameterisation...
Noel Nihill
I recently threw together the following little app for a user.
Now, before I get yelled at for talking about non-standard
functions (the POSIX thread stuff), the prototype for
'pthread_create' is:
int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
void * (*start_routine)(void *),
void *arg);
...and according to the compiler warning I'm not getting
the parameterisation quite right. The program compiles and
does what it is supposed to (if my disk LEDs are to be credited ;-))
But how can I do the Right Thing and get rid of this warning?
I admit that I am somewhat confused by a lot of the casting
above, specifically the void * (*start_routine)(void *); bit.
Could someone explain that?
<code>
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#define _REENTRANT
#define MAX_THREADS 16
int i, j, params, numdisks;
pthread_t tid[MAX_THREADS];
FILE *logfile;
typedef struct {
char *device;
int size;} argstruct, *ArgPtr;
void *thrashing(argstruct);
int main(int argc, char *argv[])
{
ArgPtr argbuf;
if(argc<3)
{
printf("Bad parameterisation - baling out...\n");
return(1);
}
params=argc; params--;
numdisks=(params/2);
j=numdisks+1;
argbuf=malloc(numdisks*sizeof(argstruct));
if (argbuf==NULL)
{
printf("malloc() failed - baling out...\n");
return(1);
}
for (i=0;i<numdisks;i++) argbuf[i].device=argv[i+1];
for (i=0;i<numdisks;i++)
{
argbuf[i].size=atoi(argv[j]);
j++;
}
for (i=0;i<numdisks;i++)
{
pthread_create(&tid[i], NULL, thrashing, &argbuf[i]); /* line 54... */
pthread_join(tid[i], NULL);
}
free(argbuf);
return(0);
}
void *thrashing(argstruct arg)
{
int i, loop, testfile;
int *buf[256];
FILE *logfile;
loop=arg.size/1024;
testfile=open (arg.device, (O_RDONLY|O_NONBLOCK));
if (testfile != -1)
{
lseek(testfile, 0, SEEK_SET);
for(i=0;i<loop;i++)
{
if (read(testfile, buf, sizeof(buf)) == -1)
{
if (logfile=fopen("alt.log", "a"))
{
fputs("Disk error from friartuck: ", logfile);
fprintf(logfile, "thread ID is %d, ", pthread_self());
fprintf(logfile, "device is %s\n", arg.device);
fflush(logfile);
fclose(logfile);
}
}
}
}
else
{
if (logfile=fopen("alt.log", "a"))
{
fputs("Error - cannot open raw device: ", logfile);
fprintf(logfile, "thread ID is %d, ", pthread_self());
fprintf(logfile, "device is %s\n", arg.device);
fflush(logfile);
fclose(logfile);
}
}
return(NULL);
}
</code>
And the compile-time stuff:
$ cc -lthread threadtuck.c
"threadtuck.c", line 54: warning: argument #3 is incompatible with prototype:
prototype: pointer to function(pointer to void) returning pointer to
void : "/usr/include/pthread.h", line 139
argument : pointer to function(struct {pointer to char device, int
size}) returning pointer to void
$
I would also invite comment on my coding style. I've been
doing it this way for quite a long time.
Your insight/mockery/conspiracy theories are most welcome.
I apologise for being slighly OT, but this is annoying me! I'd
like to develop into a bona fide expert in this game like some
of you guys.
Thanks and regards.
Noel N.
Pai-Yi HSIAO
> int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
> void * (*start_routine)(void *),
> void *arg);
>
> char *device;
> int size;} argstruct, *ArgPtr;
>
> void *thrashing(argstruct);
> pthread_create(&tid[i], NULL, thrashing, &argbuf[i]); /* line 54... */
The 3rd argument of pthread_create() requires a function pointer of type
void * (*)(void *).
But you passed a function pointer of type void *(*)(argstruct).
It's the problem!
paiyi
Noel Nihill
"Pai-Yi HSIAO" <hs...@ccr.jussieu.fr> wrote in message
news:Pine.A41.4.10.101052...@moka.ccr.jussieu.fr...
> On Wed, 23 May 2001, Noel Nihill wrote:
> > int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
> > void * (*start_routine)(void *),
> > void *arg);
> >
> > typedef struct {
> > char *device;
> > int size;} argstruct, *ArgPtr;
> >
> > void *thrashing(argstruct);
> ^^^^^^^^^
>
> > pthread_create(&tid[i], NULL, thrashing, &argbuf[i]); /* line 54... */
>
> The 3rd argument of pthread_create() requires a function pointer of type
> void * (*)(void *).
> But you passed a function pointer of type void *(*)(argstruct).
> It's the problem!
Hmmm...so how do I declare it properly? I must say I'm having
a difficult time with all this complicated declaration stuff! :0)
Tobias Oed
>
> "Pai-Yi HSIAO" <hs...@ccr.jussieu.fr> wrote in message
> news:Pine.A41.4.10.101052...@moka.ccr.jussieu.fr...
> > On Wed, 23 May 2001, Noel Nihill wrote:
> > > int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
> > > void * (*start_routine)(void *),
> > > void *arg);
> > >
> > > typedef struct {
> > > char *device;
> > > int size;} argstruct, *ArgPtr;
> > >
> > > void *thrashing(argstruct);
> > ^^^^^^^^^
> >
> > > pthread_create(&tid[i], NULL, thrashing, &argbuf[i]); /* line 54... */
> >
> > The 3rd argument of pthread_create() requires a function pointer of type
> > void * (*)(void *).
> > But you passed a function pointer of type void *(*)(argstruct).
> > It's the problem!
>
> Hmmm...so how do I declare it properly? I must say I'm having
> a difficult time with all this complicated declaration stuff! :0)
>
You should prototype
void *thrashing(void *data_p);
And then define
void *thrashing(void *data_p){
argstruct arg=*(ArgPtr) data_p;
/* bla */
}
If I understand your code correctly that is. (It compiles here without
your warning but doesn't like the #define _REENTRANT)
HTH Tobias.
Morris M. Keesan
<Noel_...@email.mot.com> wrote:
>
>"Pai-Yi HSIAO" <hs...@ccr.jussieu.fr> wrote in message
>news:Pine.A41.4.10.101052...@moka.ccr.jussieu.fr...
>> On Wed, 23 May 2001, Noel Nihill wrote:
>> > int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
>> > void * (*start_routine)(void *),
>> > void *arg);
>> >
>> > typedef struct {
>> > char *device;
>> > int size;} argstruct, *ArgPtr;
>> >
>> > void *thrashing(argstruct);
>> ^^^^^^^^^
>>
>> > pthread_create(&tid[i], NULL, thrashing, &argbuf[i]); /* line 54... */
>>
>> The 3rd argument of pthread_create() requires a function pointer of type
>> void * (*)(void *).
>> But you passed a function pointer of type void *(*)(argstruct).
>> It's the problem!
>
>Hmmm...so how do I declare it properly? I must say I'm having
>a difficult time with all this complicated declaration stuff! :0)
/* Declare thrashing() with a parameter type that matches the
* argument which will be passed to it.
*/
void *thrashing(void *vparg)
{
/* Convert the (void *) argument into a pointer to argstruct */
argstruct *argp = vparg;
/* And then replace all references to arg.something with argp->something
*/
}
Alternatively, with less change to the body of thrashing(),
void *thrashing(void *argp)
{
/* Convert (void *) argument into the right kind of pointer,
* and dereference it, copying the entire contents of the pointed-to
* object into a local object
*/
argstruct arg = *(argstruct *)argp;
/* And the rest of the function stays the same */
}
>I would also invite comment on my coding style. I've been
>doing it this way for quite a long time.
Your bracing/indenting style is a little unusual, but I don't object to
it (except in the one place where a brace was clearly misplaced,
probably by your news posting client doing the wrong thing with a tab
character).
The one thing I don't like about your style is the typedef for ArgPtr.
I find that defining a type to be a pointer to another type makes code
more difficult to read. The reader of either of my two examples can see
that the (void *) is being converted into a pointer to argstruct. With
the ArgPtr typedef, one has to search for the typedef in order to verify
that it has any relationship with type argstruct.
--
Morris Keesan -- mke...@lucent.com
Noel Nihill
"Tobias Oed" <tob...@physics.odu.edu> wrote in message
news:3B0BDE7F...@physics.odu.edu...
Just a Sun multithreading thing. Thanks for your help!
N.
>
> HTH Tobias.
Tobias Oed
I wrote:
> If I understand your code correctly that is. (It compiles here without
> your warning but doesn't like the #define _REENTRANT)
Actually the #define _REENTRANT doesn't do anything in your code. You
can
get rid of it or you may need to put it _before_ the includes where it
might
do something.
Noel Hill Wrote:
>> I would also invite comment on my coding style. I've been
>> doing it this way for quite a long time.
>>
>> Your insight/mockery/conspiracy theories are most welcome.
>> I apologise for being slighly OT, but this is annoying me! I'd
>> like to develop into a bona fide expert in this game like some
>> of you guys.
I missed that, so here are a few minor thing I don't like about your
code. You're probably aware of them, and they may have apeared
when you chopped the code down for posting.
But anyhow, here goes.
int i, j, params, numdisks;
pthread_t tid[MAX_THREADS];
FILE *logfile;
These don't need to be globals, also later you malloc argbuf, why not
do the same with tid ?
typedef struct {
char *device;
int size;} argstruct, *ArgPtr;
I don't like typedefs of data types too much and even less of pointer.
IN MAIN:
if(argc<3)
{
printf("Bad parameterisation - baling out...\n");
return(1);
}
You may also want to check that argc is odd.
Error messages should go to stderr.
There is a pair useless () arround the operand of return. And it's
a wierd retrun value of main.
params=argc; params--;
numdisks=(params/2);
j=numdisks+1;
params and j turn out to be rather useless, and this is an obfuscation
of
numdisks=(argc-1)/2;
Instead of your call to malloc you may want to use the preferred form:
argbuf=malloc(numdisks*sizeof *argbuf);
for (i=0;i<numdisks;i++) argbuf[i].device=argv[i+1];
for (i=0;i<numdisks;i++)
{
argbuf[i].size=atoi(argv[j]);
j++;
}
Why 2 loops?
for (i=0,argv++;i<numdisks;i++){
argbuf[i].device=argv[i];
argbuf[i].size=atoi(argv[i+numdisks]);
}
IN THRASHING:
int *buf[256];
I don't think you want 256 pointers to int given how you use buf later
I'd use
char buf[1024];
also, I'm guessing here but is sizeof(int *) equal to 4 on
your machine? Then
loop=arg.size/sizeof buf;
Putting this together with my previous post, and running
indent -br -ce -di1 -npsl -sob -nbap -nbad -ts1 -l200 th.c
just cause I like it that way gives:
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
struct argstruct{
char *device;
int size;
};
void *thrashing (void *data_p);
int main (int argc, char *argv[])
{
pthread_t *tid;
struct argstruct *argbuf;
int i, numdisks;
if (argc < 3 || argc % 2 != 1) {
fprintf (stderr,"Bad parameterisation - baling out...\n");
return 1;
}
numdisks = (argc-1)/2;
argbuf = malloc (numdisks * sizeof *argbuf);
tid = malloc(numdisks * sizeof *tid);
if (argbuf == NULL || tid==NULL) {
fprintf (stderr,"malloc() failed - baling out...\n");
return 1;
}
for (i = 0,argv++; i < numdisks; i++) {
argbuf[i].device = argv[i];
argbuf[i].size = atoi (argv[i + numdisks]);
}
for (i = 0; i < numdisks; i++) {
pthread_create (&tid[i], NULL, thrashing, &argbuf[i]);
pthread_join (tid[i], NULL);
}
free (argbuf);
free (tid);
return 0;
}
void *thrashing (void *data_p)
{
struct argstruct arg = *(struct argstruct *) data_p;
int i, loop, testfile;
char buf[1024];
FILE *logfile;
loop = arg.size / sizeof buf;
testfile = open (arg.device, (O_RDONLY | O_NONBLOCK));
if (testfile != -1) {
lseek (testfile, 0, SEEK_SET);
for (i = 0; i < loop; i++) {
if (read (testfile, buf, sizeof (buf)) == -1) {
if (logfile = fopen ("alt.log", "a")) {
fputs ("Disk error from friartuck: ", logfile);
fprintf (logfile, "thread ID is %d, ", pthread_self ());
fprintf (logfile, "device is %s\n", arg.device);
fflush (logfile);
fclose (logfile);
}
}
}
} else {
if (logfile = fopen ("alt.log", "a")) {
fputs ("Error - cannot open raw device: ", logfile);
fprintf (logfile, "thread ID is %d, ", pthread_self ());
fprintf (logfile, "device is %s\n", arg.device);
fflush (logfile);
fclose (logfile);
}
}
return NULL;
}
Ok now all, hammer on that piece of code, I put my hard
hat on...
Also I did not adress any of the unix specific fcn calls or
thread related stuff, cause I don't know these too well.
Tobias.
Stefan Farfeleder
>
> if (logfile = fopen ("alt.log", "a")) {
> fputs ("Disk error from friartuck: ", logfile);
> fprintf (logfile, "thread ID is %d, ", pthread_self ());
> fprintf (logfile, "device is %s\n", arg.device);
> fflush (logfile);
> fclose (logfile);
> }
More to the OP:
A call to fflush just before fclose is rather pointless IMHO. In a Real
Program, fclose's return value is worth checking.
According to my man pages pthread_self returns a value of type
pthread_t. Unless you're sure it is always an int, it might be a good
idea to cast it to int (or use long at first).
Stefan
Noel Nihill
> hat on...
> Also I did not adress any of the unix specific fcn calls or
> thread related stuff, cause I don't know these too well.
> Tobias.
Thanks very much Most constructive!
Noel.
Noel Nihill
"Stefan Farfeleder" <e002...@stud3.tuwien.ac.at> wrote in message
news:slrn9gnv0b....@stud3.tuwien.ac.at...
> In article <3B0BF2C4...@physics.odu.edu>, Tobias Oed wrote:
> >
> > if (logfile = fopen ("alt.log", "a")) {
> > fputs ("Disk error from friartuck: ", logfile);
> > fprintf (logfile, "thread ID is %d, ", pthread_self ());
> > fprintf (logfile, "device is %s\n", arg.device);
> > fflush (logfile);
> > fclose (logfile);
> > }
>
> More to the OP:
>
> A call to fflush just before fclose is rather pointless IMHO. In a Real
> Program, fclose's return value is worth checking.
That's a habit I never got into. I expect you're right.
>
> According to my man pages pthread_self returns a value of type
> pthread_t. Unless you're sure it is always an int, it might be a good
> idea to cast it to int (or use long at first).
An excellent idea, in fact. That slipped my mind.
I wrote that stuff about four months ago, and as it runs solidly
I didn't worry about the compiler until it crossed my mind
the other day. Hopefully I've improved a little since then.
Nowadays I don't do as much coding as I'd like,
so I suppose I'm beginning to go rusty. Oh well...
>
> Stefan
Tobias Oed
Made me look!
pthread_t pthread_self(void);
DESCRIPTION
This routine returns the address of the calling thread's own thread
iden-
tifier. For example, you can use this thread object to obtain the
calling
thread's own sequence number. To do so, pass the return value from
this
routine in a call to the pthread_getsequence_np(3) routine, as
follows:
unsigned long this_thread_nbr;
.
.
.
this_thread_nbr = pthread_getsequence_np( pthread_self( ) );
.
.
.
The return value from the pthread_self(3) routine becomes meaningless
after
the calling thread is destroyed.
I think we should take that over to comp.unix.programmer now...
Tobias.
Noel Nihill
>
> I wrote:
> > If I understand your code correctly that is. (It compiles here without
> > your warning but doesn't like the #define _REENTRANT)
>
> Actually the #define _REENTRANT doesn't do anything in your code. You
> can
> get rid of it or you may need to put it _before_ the includes where it
> might
> do something.
>
>
> Noel Hill Wrote:
> >> I would also invite comment on my coding style. I've been
> >> doing it this way for quite a long time.
> >>
> >> Your insight/mockery/conspiracy theories are most welcome.
> >> I apologise for being slighly OT, but this is annoying me! I'd
> >> like to develop into a bona fide expert in this game like some
> >> of you guys.
>
> I missed that, so here are a few minor thing I don't like about your
> code. You're probably aware of them, and they may have apeared
> when you chopped the code down for posting.
> But anyhow, here goes.
>
> int i, j, params, numdisks;
> pthread_t tid[MAX_THREADS];
> FILE *logfile;
Righto. I now have three questions:
1) What kind of beast is data_p?
2) Why do you say the #define _REENTRANT is redundant?
3) What is the precise meaning of things like *(struct argstruct *) data_p?
Thanks and regards,
Noel.
Pai-Yi HSIAO
> Righto. I now have three questions:
> 1) What kind of beast is data_p?
a pointer of type void *
> 2) Why do you say the #define _REENTRANT is redundant?
you never use it in your code.
> 3) What is the precise meaning of things like *(struct argstruct *) data_p?
Casting the type of data_p to "struct argstruct *" and then dereference
it.
That is, "*(struct argstruct *) data_p" is a variable of type
"struct argstruct"
paiyi
Gergo Barany
It's a pointer to void. That's a pointer type that can be implicitly
(i.e. without a cast) converted to and from any pointer-to-object
type. This is useful because functions like malloc() can return
pointers to memory without having to know exactly what type of data
you want to store in it; or because free() and memcpy() etc. can do
their magic on any type of pointer.
> 2) Why do you say the #define _REENTRANT is redundant?
It actually invades the implementation's namespace, so it invokes
undefined behavior. That undefined behavior may be that you get
exactly the effect you desire, but don't comp.lang.c complains about
such things.
> 3) What is the precise meaning of things like *(struct argstruct *) data_p?
It casts data_p, a (void *), to (struct argstruct *), so you get a
pointer value of type pointer-to-struct argstruct. The * operator
then dereferences this pointer, so you get a struct argstruct, which
in turn was used to initialize a variable of that type, IIRC.
Gergo
--
Duct tape is like the force. It has a light side, and a dark side,
and it holds the universe together ...
-- Carl Zwanzig
Noel Nihill
To Grego and Pai-Yi::
"Gergo Barany" <gergo....@gmx.net> wrote in message
news:slrn9gpr4v.d9...@hold.otthon.at...
> Noel Nihill <Noel_...@email.mot.com> wrote:
> > Righto. I now have three questions:
> >
> > 1) What kind of beast is data_p? [about void *data_p]
>
> It's a pointer to void. That's a pointer type that can be implicitly
> (i.e. without a cast) converted to and from any pointer-to-object
> type. This is useful because functions like malloc() can return
> pointers to memory without having to know exactly what type of data
> you want to store in it; or because free() and memcpy() etc. can do
> their magic on any type of pointer.
Oh I know, but I was wondering if it was some sort of standard thing.
>
> > 2) Why do you say the #define _REENTRANT is redundant?
>
> It actually invades the implementation's namespace, so it invokes
> undefined behavior. That undefined behavior may be that you get
> exactly the effect you desire, but don't comp.lang.c complains about
> such things.
[OT]: This guy is an incantation for the Sun POSIX threads library stuff.
>
> > 3) What is the precise meaning of things like *(struct argstruct *) data_p?
>
> It casts data_p, a (void *), to (struct argstruct *), so you get a
> pointer value of type pointer-to-struct argstruct. The * operator
> then dereferences this pointer, so you get a struct argstruct, which
> in turn was used to initialize a variable of that type, IIRC.
Thanks for that...;0)
Noel.
Tobias Oed
> > > 2) Why do you say the #define _REENTRANT is redundant?
> >
> > It actually invades the implementation's namespace, so it invokes
> > undefined behavior. That undefined behavior may be that you get
> > exactly the effect you desire, but don't comp.lang.c complains about
> > such things.
>
> [OT]: This guy is an incantation for the Sun POSIX threads library stuff.
>
This may very well be so.
But given the place you #defined it in your code it won't do anyting.
That's what I was complaining about, not that it invades name space.
When you define something, the preprocessor will replace any occurance
of that token by it's substitution string in the code that *follows* the
line where it was defined. Same thing for checking with #ifdef and
friends.
In your case *after* _REENTRANT gets defined (or redefined) there is no
occurance of it anywhere. So it's useless.
If you put it *before* the #include then it will be defined when these
files are included. This may then very well modify the result of the
inclusion - somehow making library/system calls reentrant.
Try to run the preprocessor on the following two codes and see if there
is
difference. (probably cc -E code1.c >code1.pre)
--- code1.c ---
#define _REENTRANT
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
--- code2.c ---
/* #define _REENTRANT */
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
Anyhow from the dec pthreads man pages:
If you use a compiler front-end or other (not C) language environment
that
does not support the -pthread compilation switch, you must use the -
D_REENTRANT compilation switch.
So my guess is that what you really want to do is take that ugly define
out
of your code and compile with
cc -D_REENTRANT th.c -lpthreads
HTH, Tobias
Micah Cowan
> In article <3B0BF2C4...@physics.odu.edu>, Tobias Oed wrote:
> >
> > if (logfile = fopen ("alt.log", "a")) {
> > fputs ("Disk error from friartuck: ", logfile);
> > fprintf (logfile, "thread ID is %d, ", pthread_self ());
> > fprintf (logfile, "device is %s\n", arg.device);
> > fflush (logfile);
> > fclose (logfile);
> > }
>
> More to the OP:
>
> A call to fflush just before fclose is rather pointless IMHO. In a Real
> Program, fclose's return value is worth checking.
Is it? I suppose that depends...
The Standard doesn't indicate what sorts of errors might occur for a
fclose() - but on the systems I'm familiar with, the only errors have
to do with basically passing in an invalid argument (FILE* which was
never opened or which has already been closed). It is conceivable
that a system could have an open file which it is unable to close for
one reason or another - but in that event, what should be the
programmer's response?
The only real value *I* see (perhaps you can enlighten me further?) in
using the return value is something like:
assert (fclose(stream) != EOF);
(which is only sure to be a good idea on systems like mine where it
indicates bad arguments), or:
#ifdef DEBUG
if (fclose(stream))
{
perror ("Couldn't close a file");
fprintf ("This may mean that you've passed an invalid stream,\n"
"so for your information, it occured on line %s of file %s.\n",
__LINE__, __FILE__);
}
#endif
But such constructs ought not to be thrown all over the code - I use
asserts to check for "impossibilities", but not in every place - only
ones where I have the slightest belief that I could make a serious
mistake. I have never yet had a problem which would be resolved by
checking fclose()'s retval, nor have I ever heard of someone else
having one, and this is one place I doubt I'll ever bother to check.
Micah
--
The way of a fool is right in his own eyes, but he who
heeds counsel is wise.
Proverbs 12:15
Mark McIntyre
<tob...@physics.odu.edu> wrote:
>> >
>> > > 2) Why do you say the #define _REENTRANT is redundant?
>> >
>> > It actually invades the implementation's namespace, so it invokes
>> > undefined behavior. That undefined behavior may be that you get
>> > exactly the effect you desire, but don't comp.lang.c complains about
>> > such things.
>>
>> [OT]: This guy is an incantation for the Sun POSIX threads library stuff.
>>
>When you define something, the preprocessor will replace any occurance
>of that token by it's substitution string in the code that *follows* the
>line where it was defined. Same thing for checking with #ifdef and
>friends.
Howver it can also be used in conditional compiling. I don't see the
original code but there's nothing to stop it having significant
effects if for example one of the functions called is in fact a
function-like macro, and two versions of that macro exist.
>In your case *after* _REENTRANT gets defined (or redefined) there is no
>occurance of it anywhere. So it's useless.
As a preprocessor text replacement value, yes. Typically tho, #defines
with no specified value tend to be conditional compile flags.
//file1.c
#define _REENTRANT
#include "file2.h"
int main(void)
{
Produce("Hello");
}
//file2.h
#if defined(_REENTRANT)
#define Produce(xx) remove(xx)
#else
int puts(char*);
#define Produce(xx) puts(xx)
#endif
--
Mark McIntyre
CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
Eric Sosman
>
> The Standard doesn't indicate what sorts of errors might occur for a
> fclose() - but on the systems I'm familiar with, the only errors have
> to do with basically passing in an invalid argument (FILE* which was
> never opened or which has already been closed). It is conceivable
> that a system could have an open file which it is unable to close for
> one reason or another - but in that event, what should be the
> programmer's response?
fclose() can most assuredly fail, even if the argument is
perfectly valid and the program hasn't done anything wrong. I
recall a program which meticulously checked the return value of
every fwrite() call, but neglected to check what fclose() said;
that program wound up costing my employer a non-trivial amount
of money and the loss of some customer good-will ...
The program sucked in an existing file and massaged the data
in response to user input over a period of hours (it was a kind
of document editor). Every so often the program would dump the
current state of the document to a checkpoint file, just in case
anything should go wrong. Finally, the user told the program to
save the final document, and this happened:
Write everything to a temporary file, being very careful
to check each fwrite().
If all the writes succeeded (they had)
{
fclose() the temporary file. Unfortunately, the disk
filled up, the last few in-memory buffers never
got written, and the failure went undetected.
remove() the original file.
rename() the temporary file to the original's name.
remove() the checkpoint file -- it's no longer needed
because we've just completed a "successful" save.
}
Truncated files were useless; the program couldn't reconstruct
the document in the absence of part of the data. So the user's
entire editing session was irretrievably lost, and as Murphy would
have it, this particular user happened to be the Big Cheese at a Big
Customer Site, a person who was already rather unhappy with us for
other reasons. We had to fly some High Company Officials to the site
to mollify him and cut him a sweetheart deal on upgrades to keep him
from throwing us out on our, er, ear.
So in answer to your question "What should be the programmer's
response?" I'd say that some response other than the one actually
taken would have saved considerable grief and a pile of cash. All
for the want of an `if'.
Michael Rubenstein
wrote:
Most implementatsions (read: every one I'm familiar with) may
also return an error if the file is going to disk and writing
buffered data fails at fclose() because the disk is full.
--
Michael M Rubenstein
CBFalconer
>
> mistake. I have never yet had a problem which would be resolved by
> checking fclose()'s retval, nor have I ever heard of someone else
> having one, and this is one place I doubt I'll ever bother to check.
One scenario that occurs to me is writing a file on removable
media, and the operator ejects that before the close (and final
flush). If the data is precious the opportunity to retry could be
very valuable.
--
Chuck F (cbfal...@my-deja.com) (cbfal...@XXXXworldnet.att.net)
http://www.qwikpages.com/backstreets/cbfalconer :=(down for now)
(Remove "NOSPAM." from reply address. my-deja works unmodified)
mailto:u...@ftc.gov (for spambots to harvest)
Stefan Farfeleder
>e002...@stud3.tuwien.ac.at (Stefan Farfeleder) writes:
>
>> In article <3B0BF2C4...@physics.odu.edu>, Tobias Oed wrote:
>> >
>> > if (logfile = fopen ("alt.log", "a")) {
>> > fputs ("Disk error from friartuck: ", logfile);
>> > fprintf (logfile, "thread ID is %d, ", pthread_self ());
>> > fprintf (logfile, "device is %s\n", arg.device);
>> > fflush (logfile);
>> > fclose (logfile);
>> > }
>>
>> More to the OP:
>>
>> A call to fflush just before fclose is rather pointless IMHO. In a Real
>> Program, fclose's return value is worth checking.
>
>Is it? I suppose that depends...
>
>The Standard doesn't indicate what sorts of errors might occur for a
>fclose() - but on the systems I'm familiar with, the only errors have
>to do with basically passing in an invalid argument (FILE* which was
>never opened or which has already been closed). It is conceivable
>that a system could have an open file which it is unable to close for
>one reason or another - but in that event, what should be the
>programmer's response?
Reasons for failure are mostly full filesystems or disk errors, I
suppose. You could dump the information that otherwise should have been
printed to the logfile to stderr (which will probably fail too if it is
redirected to a file).
>The only real value *I* see (perhaps you can enlighten me further?) in
>using the return value is something like:
>
>assert (fclose(stream) != EOF);
>
>(which is only sure to be a good idea on systems like mine where it
>indicates bad arguments), or:
>
>#ifdef DEBUG
>if (fclose(stream))
> {
> perror ("Couldn't close a file");
> fprintf ("This may mean that you've passed an invalid stream,\n"
> "so for your information, it occured on line %s of file %s.\n",
> __LINE__, __FILE__);
> }
>#endif
And if NDEBUG respectively DEBUG are defined, you don't want your stream
to be closed?
Stefan
Stefan Farfeleder
Stefan Farfeleder wrote:
>>
>>assert (fclose(stream) != EOF);
>>
>>(which is only sure to be a good idea on systems like mine where it
>>indicates bad arguments), or:
>>
>>#ifdef DEBUG
>>if (fclose(stream))
>> {
>> perror ("Couldn't close a file");
>> fprintf ("This may mean that you've passed an invalid stream,\n"
>> "so for your information, it occured on line %s of file %s.\n",
>> __LINE__, __FILE__);
>> }
>>#endif
>
>And if NDEBUG respectively DEBUG are defined, you don't want your stream
>to be closed?
What I meant was: if NDEBUG is defined or DEBUG is not defined, ...
Stefan
Micah Cowan
> >The only real value *I* see (perhaps you can enlighten me further?) in
> >using the return value is something like:
> >
> >assert (fclose(stream) != EOF);
> >
> >(which is only sure to be a good idea on systems like mine where it
> >indicates bad arguments), or:
> >
> >#ifdef DEBUG
> >if (fclose(stream))
> > {
> > perror ("Couldn't close a file");
> > fprintf ("This may mean that you've passed an invalid stream,\n"
> > "so for your information, it occured on line %s of file %s.\n",
> > __LINE__, __FILE__);
> > }
> >#endif
>
> And if NDEBUG respectively DEBUG are defined, you don't want your stream
> to be closed?
They were for illustration purposes only, obviously.
But the points that have been raised so far are good ones, so I
obviously must rethink my rather rash position.
Still, I ask - how should they be handled? For now the program should
never exit until something is done to increase disk space or whatnot,
for fear of losing precious data? Or should it exit anyway, with a
bland error message informing you that you just lost your data, and
why? ;)
Micah
--
A wise man will hear, and increase learning; and a man of
understanding will attain wise counsel.
Proverbs 1:5
Stefan Farfeleder
>But the points that have been raised so far are good ones, so I
>obviously must rethink my rather rash position.
>
>Still, I ask - how should they be handled? For now the program should
>never exit until something is done to increase disk space or whatnot,
>for fear of losing precious data? Or should it exit anyway, with a
>bland error message informing you that you just lost your data, and
>why? ;)
Yes ;-)
Stefan
Mark
> Micah Cowan wrote:
> > mistake. I have never yet had a problem which would be resolved by
> > checking fclose()'s retval, nor have I ever heard of someone else
> > having one, and this is one place I doubt I'll ever bother to check.
> One scenario that occurs to me is writing a file on removable
> media, and the operator ejects that before the close (and final
> flush). If the data is precious the opportunity to retry could be
> very valuable.
The standard only states that fclose() must return EOF on failure,
mentions nothing about being able to retry. Some OS's (such as
FreeBSD) don't allow further access to the stream if fclose() fails,
and attempting to fclose() the stream a second time would invoke
undefined behaviour.
Regards,
Mark