Pse, help me to understand this code

[__frank__]

The following code use a macro and a label.

I would to change it and use instead
a more readable function and avoid the label.

The macro DAQmxFailed checks for the return code
of the various functions:
DAQmxCreateTask()
DAQmxStartTask()
DAQmxReadAnalogScalarF64()
and others one

If these functions return a int < 0 means
that an error occured so DAQmxGetExtendedErrorInfo
displays info about error.

DAQmxFailed is another macro (defined in NIDAQmx.h):

#define DAQmxFailed(error) ((error)<0)

Taskhandle is a typedef of unsigned long;
float 64 of a __int64 (MS VC++); int32 is a long

Thanks in advance

/***************** Begin ******************* /

#include <stdio.h>
#include "NIDAQmx.h"

#define DAQmxErrChk(functionCall) { if(
DAQmxFailed(error=(functionCall)) ) { goto Error; } }

int main(int argc, char *argv[])
{
int32 error=0;
TaskHandle taskHandle;
float64 value;
char errBuff[2048]={'\0'};

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/
DAQmxErrChk (DAQmxCreateTask("",&taskHandle));
DAQmxErrChk
(DAQmxCreateAIVoltageChan(taskHandle,"Dev1/ai0","",DAQmx_Val_Cfg_Default,-10.0,10.0,DAQmx_Val_Volts,""));

/*********************************************/
/*/ DAQmx Start Code
/*********************************************/
DAQmxErrChk (DAQmxStartTask(taskHandle));

/*********************************************/
/*/ DAQmx Read Code
/*********************************************/
DAQmxErrChk (DAQmxReadAnalogScalarF64(taskHandle,10.0,&value,0 ));

printf("Acquired reading: %f\n",value);

Error:
if( DAQmxFailed (error) )
DAQmxGetExtendedErrorInfo(errBuff,2048);
if( taskHandle!=0 ) {
/*********************************************/
/*/ DAQmx Stop Code
/*********************************************/
DAQmxStopTask(taskHandle);
DAQmxClearTask(taskHandle);
}
if( DAQmxFailed (error) )
printf("DAQmx Error: %s\n",errBuff);
printf("End of program, press Enter key to quit\n");
getchar();
return 0;
}

[Flash Gordon]

__frank__ wrote:

The following code use a macro and a label.

I would to change it and use instead
a more readable function and avoid the label.
In general I am anti-goto, but for handling exceptional conditions it
can be useful, and that is how it is used here. My main complaint would
be hiding the control statement (goto) within the macro.
The macro DAQmxFailed checks for the return code
of the various functions:
DAQmxCreateTask()
DAQmxStartTask()
DAQmxReadAnalogScalarF64()
and others one

If these functions return a int < 0 means
that an error occured so DAQmxGetExtendedErrorInfo
displays info about error.

DAQmxFailed is another macro (defined in NIDAQmx.h):

#define DAQmxFailed(error) ((error)<0)
Traditionally macros are all upper case, e.g.
#define DAQMXFAILED(error) ((error)<0)

This makes it stand out which is important because macros can have
non-obvious effects.
Taskhandle is a typedef of unsigned long;
float 64 of a __int64 (MS VC++); int32 is a long

Thanks in advance

/***************** Begin ******************* /

#include <stdio.h>
#include "NIDAQmx.h"

#define DAQmxErrChk(functionCall) { if( DAQmxFailed(error=(functionCall)) ) { goto Error; } }

int main(int argc, char *argv[])
{
int32 error=0;
TaskHandle taskHandle;
float64 value;
char errBuff[2048]={'\0'};
do {
/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/
DAQmxErrChk (DAQmxCreateTask("",&taskHandle));
error = QmxCreateTask("",&taskHandle);
if (error < 0)
break;
DAQmxErrChk
(DAQmxCreateAIVoltageChan(taskHandle,"Dev1/ai0","",DAQmx_Val_Cfg_Default,-10.0,10.0,DAQmx_Val_Volts,""));
error = ......
if (error < 0)
break;
/*********************************************/
/*/ DAQmx Start Code
/*********************************************/
DAQmxErrChk (DAQmxStartTask(taskHandle));
error = DAQmxStartTask(taskHandle);
if (error < 0)
break;
/*********************************************/
/*/ DAQmx Read Code
/*********************************************/
DAQmxErrChk (DAQmxReadAnalogScalarF64(taskHandle,10.0,&value,0 ));
error = ......
if (error < 0)
break;
printf("Acquired reading: %f\n",value);
} while (0);
Error:
The problem with using a loop and break is when add in other control
structures such as other loops and switch statements since the break
only takes you out of the top most.

Another alternative is to use two functions, one which does the work and
returns on error and the second that calls it and handles clean up. Then
you can do clever things like

int worker(......)
{
... do stuff
if (....)
return -1;
... do stuff
if (....)
return -2;
......
}

int controller(......)
{
int ret = worker(......)
switch (ret) {
case 0: .....
case -1: ....
......
}

Sometimes you would not want breaks between the cases in the above
switch (e.g. if it is just doing clean up) sometimes you would.

Remember that beauty is in the eye of the beholder, so whatever method
you use some people will dislike it. I've used several different methods
depending on the exact circumstances and what I thought would be best in
each case.
if( DAQmxFailed (error) )
if (error < 0)
DAQmxGetExtendedErrorInfo(errBuff,2048);
if( taskHandle!=0 ) {
/*********************************************/
/*/ DAQmx Stop Code
/*********************************************/
DAQmxStopTask(taskHandle);
DAQmxClearTask(taskHandle);
}
if( DAQmxFailed (error) )
printf("DAQmx Error: %s\n",errBuff);
printf("End of program, press Enter key to quit\n");
getchar();
return 0;
}

--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Richard Bos]

Flash Gordon <sp**@flash-gordon.me.uk> wrote:

__frank__ wrote:

The following code use a macro and a label.

I would to change it and use instead
a more readable function and avoid the label.
In general I am anti-goto, but for handling exceptional conditions it
can be useful, and that is how it is used here. My main complaint would
be hiding the control statement (goto) within the macro.

I agree. Therefore...
do {

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/
DAQmxErrChk (DAQmxCreateTask("",&taskHandle));

error = QmxCreateTask("",&taskHandle);
if (error < 0)
break;

printf("Acquired reading: %f\n",value);

} while (0);

....this is evil, since you're still in effect using a goto, but you've
hidden it inside a bogus loop construct.
If you _must_ use goto (and sometimes you must, or at least it's the
least bad choice), be up-front about it.

Richard

[Jordan Abel]

On 2005-10-28, __frank__ <no***********@indirizzo.non.valido> wrote:

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/

What is this? You do realize that if you did a multiline comment in
this style every second line wouldn't be commented, right?

[__frank__]

Jordan Abel ha scritto:

What is this? You do realize that if you did a multiline comment in
this style every second line wouldn't be commented, right?

Just a copy&paste from a sources provided (together the
acquisition card) with the installation of NI-DAQmx
(drivers/software for NI data-acquistion cards).

[__frank__]

Thanks for the preciuos help.
This afternoon I'll try to build first using
the sources provided from the manufacturer (that
seems to be bad written - see Jordan's post below)
then using sources with your suggestions.
Regards,
Frank

[Flash Gordon]

Richard Bos wrote:

Flash Gordon <sp**@flash-gordon.me.uk> wrote:

__frank__ wrote:

The following code use a macro and a label.

I would to change it and use instead
a more readable function and avoid the label.

^^^^^^^^^^^^^^^

In general I am anti-goto, but for handling exceptional conditions it
can be useful, and that is how it is used here. My main complaint would
be hiding the control statement (goto) within the macro.

I agree. Therefore...

do {

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/
DAQmxErrChk (DAQmxCreateTask("",&taskHandle));

error = QmxCreateTask("",&taskHandle);
if (error < 0)
break;

printf("Acquired reading: %f\n",value);

} while (0);

...this is evil, since you're still in effect using a goto, but you've
hidden it inside a bogus loop construct.
If you _must_ use goto (and sometimes you must, or at least it's the
least bad choice), be up-front about it.

The OP wanted to avoid the label, so I told him how it could be done. I
also pointed out some of the problem.

The problem with the goto approach is that, on seeing the label, you
have no bounds on how much of the function you need to check. With the
do loop at least you know the scope is limited to that block within the
function.

I consider neither approach to be ideal and which C had the very limited
exception handling that HP Pascal had, where you could throw an integer
error code and catch it. IIRC it was

TRY
do stuff
IF whatever THEN
ESCAPE(-1)
do stuff
RECOVER BEGIN
do recovery (you have access to what the code was, obviously)
IF not fully handled THEN
ESCAPE(whatever)
END;

If ESCAPE was called when you were not in a TRY RECOVER block it acted
like the C abort call.

Even a more limited form of this where you could only use "escape"
within a "try recover" block (just like you can only use continue within
a loop) would help a lot IMHO especially as I know some people don't
like the idea of an exception automatically propagating up several
layers before it is caught.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[__frank__]

What do you think about following DAQmxErrChk ?
Parameter of DAQmxErrChk is the return code (status)
of function:
int32 DAQmxErrChk (int32 status)

char errBuff[2048];

{
if (status < 0)

{
DAQmxGetExtendedErrorInfo(errBuff,2048);
printf("DAQmx Error: %s\n",errBuff);
exit(1)
}

}

[__frank__]

Got a warning, but it seems work:

warning C4715: 'not all control paths return a value

[Flash Gordon]

__frank__ wrote:

Got a warning, but it seems work:

warning C4715: 'not all control paths return a value

Haven't you seen the posts to this very group over the past two days
about the need to provide context? Since Thunderbird provides you the
context by default I'm not sure why you felt the need to delete it all
making your post illegible to anyone who happens not to have received
the article you are replying to.

As to the warning, it probably means exactly what it says. You have
written a function that is defined as returning a value but does not
under some conditions.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Flash Gordon]

__frank__ wrote:

What do you think about following DAQmxErrChk ?
In reference to what? Leave in some context when replying so people can
see what you are talking about.
Parameter of DAQmxErrChk is the return code (status)
of function:

int32 DAQmxErrChk (int32 status)

char errBuff[2048];

{

Even given a definition of int32 I think that this is not valid C. Local
variables should be declared after the opening brace not before it.I did
not read further.

<snip>
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Christopher Benson-Manica]

Flash Gordon <sp**@flash-gordon.me.uk> wrote:

As to the warning, it probably means exactly what it says. You have
written a function that is defined as returning a value but does not
under some conditions.

It's probably not this, but I would think there is a possibility that
a crappy compiler might issue such a warning for the following code,
which is similar to OP's:

int myfunc( void ) {
if( /*...*/ ) {
exit( 0 );
}
else {
return 0;
}
}

If it isn't smart enough to recognize that the call to exit()
constitutes a "return" of sorts, it might complain as OP's compiler
did.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

[__frank__]

Flash Gordon ha scritto:

Even given a definition of int32 I think that this is not valid C. Local
variables should be declared after the opening brace not before it.

Ok. Please, let me know what do you think about the following code.
Compiling I got no errors, no warnings:

/************************************************** ***/
int DAQmxErrChk (int32 status)
{

char errBuff[2048]={'\0'};

if (status < 0)
{

DAQmxGetExtendedErrorInfo(errBuff,2048);
printf("DAQmx Error: %s\n",errBuff);
return 1;
}

else
return 0; /* No error */

}

[Flash Gordon]

__frank__ wrote:

<snip>

Ok. Please, let me know what do you think about the following code.
Compiling I got no errors, no warnings:

/************************************************** ***/
int DAQmxErrChk (int32 status)
Why don't you just use int? Do status codes *really* require 32 bits
(which is what I assume int32 is)?
{

char errBuff[2048]={'\0'};
Did you realise that this would initialise *all* 2048 bytes of errBuff?
This is probably a waste, although it won't stop the function from working.
if (status < 0)
{

DAQmxGetExtendedErrorInfo(errBuff,2048);
Since, I presume, the above function always writes a string to the
provided buffer I would be surprised if errBuff actually needed any
initialisation.
printf("DAQmx Error: %s\n",errBuff);
It is generally useful to send error messages to stderr (the standard
error stream) instead of stdout.
fprintf(stderr,"DAQmx Error: %s\n",errBuff);
return 1;
}

else
return 0; /* No error */

}

The function should work as far as I can see, assuming sensible
definitions of int32, DAQmxGetExtendedErrorInfo and inclusion of stdio.h
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[__frank__]

Flash Gordon ha scritto:

Why don't you just use int? Do status codes *really* require 32 bits
(which is what I assume int32 is)?
The variuos DAQmx are defined to return a "int32" (typedef of long in
NIDAQmx.h) so I would to use these typedef.

fprintf(stderr,"DAQmx Error: %s\n",errBuff);
Right, it's better.

The function should work as far as I can see

So far so good. Hope to make a little improved source.

Thanks again

[__frank__]

Flash Gordon ha scritto:

Why don't you just use int? Do status codes *really* require 32 bits
(which is what I assume int32 is)?

Here you are examples of status codes (printed by mean of
DAQmxGetExtendedErrorInfo):
Task Name: voltage1

Status Code: -200220
DAQmx Error: Specified operation cannot be performed when there are no
channels
in the task.
Task Name: voltage1

Status Code: -200478
DAQmx Error: Specified operation cannot be performed when there are no
channels
in the task.
Task Name: voltage1

Status Code: -200478

[Keith Thompson]

Jordan Abel <jm****@purdue.edu> writes:

On 2005-10-28, __frank__ <no***********@indirizzo.non.valido> wrote:

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/

What is this? You do realize that if you did a multiline comment in
this style every second line wouldn't be commented, right?

Actually, that's not correct. Of the three lines, the first is a
valid comment. The leading "/*" on the second line introduces a
second comment, which is terminated by the trailing "*/" on the third
line.

It's confusing because there's a "*/" on the second line that doesn't
close a comment (because the '*' is part of the "/*" that introduces
it), and a "/*" on the third line that doesn't introduce a comment
(because it's already within a comment, and comments don't nest).

It's horrible style, requiring too much effort by the reader to
confirm that it's valid, but it's not actually wrong.

Either of the following would be better:

/*
* DAQmx Configure Code
*/

or

/*********************************************/
/* DAQmx Configure Code */
/*********************************************/

--
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.

[Jordan Abel]

On 2005-10-30, Keith Thompson <ks***@mib.org> wrote:

Jordan Abel <jm****@purdue.edu> writes:

On 2005-10-28, __frank__ <no***********@indirizzo.non.valido> wrote:

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/
What is this? You do realize that if you did a multiline comment in
this style every second line wouldn't be commented, right?

Actually, that's not correct. Of the three lines, the first is a
valid comment. The leading "/*" on the second line introduces a
second comment, which is terminated by the trailing "*/" on the third
line.

It's confusing because there's a "*/" on the second line that doesn't
close a comment (because the '*' is part of the "/*" that introduces
it), and a "/*" on the third line that doesn't introduce a comment
(because it's already within a comment, and comments don't nest).

That's what i was trying to say - _this_ happens to be a valid
comment (well, two comments) but if he tried to do

/******/ /*/ 1 /*/ 2 /*/ 3 /*/ 4 /******/
it would become
/*....*/ /*.....*/ 2 /*.....*/ 4 /*....*/

and
/******/ /*/ 1 /*/ 2 /*/ 3 /******/
becomes
/*....*/ /*.....*/ 2 /*..........*/
It's horrible style, requiring too much effort by the reader to
confirm that it's valid, but it's not actually wrong.

[Neil Cerutti]

On 2005-10-30, Keith Thompson <ks***@mib.org> wrote:

Jordan Abel <jm****@purdue.edu> writes:

On 2005-10-28, __frank__ <no***********@indirizzo.non.valido> wrote:

/*********************************************/
/*/ DAQmx Configure Code
/*********************************************/

Actually, that's not correct. Of the three lines, the first is a
valid comment. The leading "/*" on the second line introduces a
second comment, which is terminated by the trailing "*/" on the third
line.

It's confusing because there's a "*/" on the second line that doesn't
close a comment (because the '*' is part of the "/*" that introduces
it), and a "/*" on the third line that doesn't introduce a comment
(because it's already within a comment, and comments don't nest).

It's horrible style, requiring too much effort by the reader to
confirm that it's valid, but it's not actually wrong.

Either of the following would be better:

/*
* DAQmx Configure Code
*/

or

/*********************************************/
/* DAQmx Configure Code */
/*********************************************/

Or the fantabulous:

/************************************************** *******/
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* DAQmx Configure Code!!! * * * * * * * * * * * * * * * */
/* * * * DAQmx Configure Code!!! * * * * * * * * * * * * */
/* * * * * * * * DAQmx Configure Code!!! * * * * * * * * */
/* * * * * * * * * * * * DAQmx Configure Code!!! * * * * */
/* * * * * * * * * * * * * ** * *DAQmx Configure Code!!! */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/************************************************** *******/

But seriously, comment decorations are a waste. They add very
little and are a pain to maintain.

/* DAQmx Configure Code */

--
Neil Cerutti