Pse, help me to understand this code

Discussion in 'C Programming' started by __frank__, Oct 28, 2005.

  1. __frank__

    __frank__ Guest

    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;
    }
    __frank__, Oct 28, 2005
    #1
    1. Advertising

  2. __frank__

    Flash Gordon Guest

    __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.
    Flash Gordon, Oct 28, 2005
    #2
    1. Advertising

  3. __frank__

    Richard Bos Guest

    Flash Gordon <> 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
    Richard Bos, Oct 28, 2005
    #3
  4. __frank__

    Jordan Abel Guest

    On 2005-10-28, __frank__ <> 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?
    Jordan Abel, Oct 28, 2005
    #4
  5. __frank__

    __frank__ Guest

    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__, Oct 28, 2005
    #5
  6. __frank__

    __frank__ Guest

    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
    __frank__, Oct 28, 2005
    #6
  7. __frank__

    Flash Gordon Guest

    Richard Bos wrote:
    > Flash Gordon <> 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.
    Flash Gordon, Oct 28, 2005
    #7
  8. __frank__

    __frank__ Guest

    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__, Oct 28, 2005
    #8
  9. __frank__

    __frank__ Guest

    Got a warning, but it seems work:

    warning C4715: 'not all control paths return a value
    __frank__, Oct 28, 2005
    #9
  10. __frank__

    Flash Gordon Guest

    __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, Oct 28, 2005
    #10
  11. __frank__

    Flash Gordon Guest

    __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.
    Flash Gordon, Oct 28, 2005
    #11
  12. Flash Gordon <> 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.
    Christopher Benson-Manica, Oct 28, 2005
    #12
  13. __frank__

    __frank__ Guest

    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 */

    }
    __frank__, Oct 29, 2005
    #13
  14. __frank__

    Flash Gordon Guest

    __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.
    Flash Gordon, Oct 29, 2005
    #14
  15. __frank__

    __frank__ Guest

    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__, Oct 29, 2005
    #15
  16. __frank__

    __frank__ Guest

    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
    __frank__, Oct 29, 2005
    #16
  17. Jordan Abel <> writes:
    > On 2005-10-28, __frank__ <> 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) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    We must do something. This is something. Therefore, we must do this.
    Keith Thompson, Oct 30, 2005
    #17
  18. __frank__

    Jordan Abel Guest

    On 2005-10-30, Keith Thompson <> wrote:
    > Jordan Abel <> writes:
    >> On 2005-10-28, __frank__ <> 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.
    Jordan Abel, Oct 30, 2005
    #18
  19. __frank__

    Neil Cerutti Guest

    On 2005-10-30, Keith Thompson <> wrote:
    > Jordan Abel <> writes:
    >> On 2005-10-28, __frank__ <> 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
    Neil Cerutti, Oct 31, 2005
    #19
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Ketul Patel
    Replies:
    1
    Views:
    717
    Alvin Bruney
    Nov 29, 2003
  2. Tobi Krausl

    JComboBox problem, pse help!

    Tobi Krausl, Nov 27, 2003, in forum: Java
    Replies:
    6
    Views:
    4,188
    Tobi Krausl
    Dec 1, 2003
  3. salvatore
    Replies:
    0
    Views:
    469
    salvatore
    Jan 23, 2005
  4. Eduardo Dobay

    Using PSE under Win32

    Eduardo Dobay, Jun 23, 2007, in forum: Python
    Replies:
    2
    Views:
    293
    Eduardo Dobay
    Jun 26, 2007
  5. IgorB
    Replies:
    0
    Views:
    160
    IgorB
    Jun 5, 2006
Loading...

Share This Page