3 short but not so sweet...



  • heres 3 short examples of strange coding (C) of a program i am currently having to go through:

    // Isnt this an if statement???

    switch (msg.INFO_TIMER_TAG)
    {
        case ROOT_TIMER_ID:
            rootTimerCallback(_root);
            bCallDispatch = FALSE;
            break;
    }

    // Shouldnt this be an else???

    static void com_resetPingTimer()
    {
        if(_state.tmrPing == ILLEGAL_HANDLE)
        {
            _state.tmrPing = TML_registerTimerCallbackHandle(com_pingTimerCallback, NULL);
        }

        if(_state.tmrPing != ILLEGAL_HANDLE)
        {
            O_timer_start(_state.tmrPing, PING_TIMEOUT);
        }
    }


    // passing a param for the fun of it...?

    static void com_HttpConnecting(int cond)
    {
        _state.modemState = MODEM_STATE_CONNECTING;
    }



  • @dtvdeveloper said:

    // Shouldnt this be an else???

    static void com_resetPingTimer()
    {
        if(_state.tmrPing == ILLEGAL_HANDLE)
        {
            _state.tmrPing = TML_registerTimerCallbackHandle(com_pingTimerCallback, NULL);
        }

        if(_state.tmrPing != ILLEGAL_HANDLE)
        {
            O_timer_start(_state.tmrPing, PING_TIMEOUT);
        }
    }

    In theory TML_registerTimerCallbackHandle() (I don't recognise it, so I can't be sure) could return ILLEGAL_HANDLE. Then this is valid code to make sure it didn't.



  • @dtvdeveloper said:

    heres 3 short examples of strange coding (C) of a program i am currently having to go through:

    // Isnt this an if statement???

    switch (msg.INFO_TIMER_TAG)
    {
        case ROOT_TIMER_ID:
            rootTimerCallback(_root);
            bCallDispatch = FALSE;
            break;
    }




    Maybe, maybe not.   If there is reason to believe that many
    other cases will need to be handled in the future, then no.  
    In fact I'm be inclined to use this construct if there are many
    possibilities, because a switch indicates better that there are many
    possibilities.



    @dtvdeveloper said:



    // Shouldnt this be an else???

    static void com_resetPingTimer()
    {
        if(_state.tmrPing == ILLEGAL_HANDLE)
        {
            _state.tmrPing = TML_registerTimerCallbackHandle(com_pingTimerCallback, NULL);
        }

        if(_state.tmrPing != ILLEGAL_HANDLE)
        {
            O_timer_start(_state.tmrPing, PING_TIMEOUT);
        }
    }


    As the other guy said, no.   If it was an else it would be:



    static void com_resetPingTimer()

    {

        if(_state.tmrPing == ILLEGAL_HANDLE)

        {

            _state.tmrPing = TML_registerTimerCallbackHandle(com_pingTimerCallback, NULL);

          if(_state.tmrPing != ILLEGAL_HANDLE)

          {

              O_timer_start(_state.tmrPing, PING_TIMEOUT);

          }

      }

      else

      {

         O_timer_start(_state.tmrPing, PING_TIMEOUT);

      }

    }



    Clearly the above is a WTF

    @dtvdeveloper said:


    // passing a param for the fun of it...?

    static void com_HttpConnecting(int cond)
    {
        _state.modemState = MODEM_STATE_CONNECTING;
    }




    If this is part of a class,. then no, because a child method may
    override this, and need the parameter.   In fact it looks
    like this is the case.



    You implied C, which doesn't have classes, but if that is the case look for a function like this:



    static void net_HttpConnection(int cont)



      if (cond == foo)  bar();

      else baz();

    }



    The int parameter is needed because if you use a function pointer to
    switch between modem and network code, you need to have the unused
    parameter so the pointers work right.



    Often too I have refactored a function to not need a parameter, but
    decided the effort to remove the unused parameter is not worth the
    bother, which would again leave something like this.



    Note that if this is C, variables that begin with an _ are often
    reserved (the rules are a little tricky, I can't remember them exactly).



  • duh! yeah your right, cheers for the correction




  • @dtvdeveloper said:



    // passing a param for the fun of it...?

    static void com_HttpConnecting(int cond)
    {
        _state.modemState = MODEM_STATE_CONNECTING;
    }



    If this is part of a class,. then no, because a child method may
    override this, and need the parameter.   In fact it looks
    like this is the case.



    You implied C, which doesn't have classes, but if that is the case look for a function like this:



    static void net_HttpConnection(int cont)



      if (cond == foo)  bar();

      else baz();

    }



    The int parameter is needed because if you use a function pointer to
    switch between modem and network code, you need to have the unused
    parameter so the pointers work right.



    Often too I have refactored a function to not need a parameter, but
    decided the effort to remove the unused parameter is not worth the
    bother, which would again leave something like this.



    Note that if this is C, variables that begin with an _ are often
    reserved (the rules are a little tricky, I can't remember them exactly).


    Its C (not C++), so no objects or overloading.  Interesting your point about using a pointer to the function, I did not think it would care that the parameters would need to be the same as the function is itself a pointer.....(so in essence it would be just a pointer to a pointer wouldnt it?)

    C vars with _ prefixes being reserved is definitely not in this case.  i take it that must be a platform specific rule? (i have never heard that or read it in any book).



  • @dtvdeveloper said:



    Its C (not C++), so no objects or overloading.  Interesting your point about using a pointer to the function, I did not think it would care that the parameters would need to be the same as the function is itself a pointer.....(so in essence it would be just a pointer to a pointer wouldnt it?)

    If your functions don't have the same parameter list, your calling code has to be written to either pass or not pass a parameter based on which function it's calling ... at which point, the use of the function pointers is moot.

    So in the prior example, your calling code would need to explicitly call either net_HttpConnection with a parameter or com_HttpConnecting without a parameter, rather than calling an anonymous function with a parameter that's used (or not) by the actual function that is invoked.

     

     


  • @dtvdeveloper said:


    C vars with _ prefixes being reserved is
    definitely not in this case.  i take it that must be a platform
    specific rule? (i have never heard that or read it in any book).




    I don't have a copy of the standard, but GNU follows it closely enough.




    In addition to the names documented in this manual, reserved names
    include all external identifiers (global functions and variables) that
    begin with an underscore (`<samp>_</samp>') and all identifiers regardless of
    use that begin with either two underscores or an underscore followed by
    a capital letter are reserved names. This is so that the library and
    header files can define functions, variables, and macros for internal
    purposes without risk of conflict with names in user programs.



    I did read much the same in the C++ standard, but I no longer have access to it, so I can't give you the section.



    Note that nothing is implied about how things happen in the real world
    if you use them.  I've seen external identifiers that begin with a
    _ work just fine on some systems, and fail on others.  I've been
    burned by this, so I fell obligated to point it out to everyone who
    uses C, in hopes that they fix their code so I'm not burned
    again.  (And along the way keep them from getting burned)



  • OMFG

    That is O-code (aka OpenTV VM) isn't it?

    I am still somewhat immersed in that stuff...



    Dude, if you want to see some WTF's, you only have to look through "opentv.h" - it looks to me like the developer of the above simply followed the standard set by the creators.



    Here's an example from opentv.h;

    #define FALSE (0)
    #define TRUE  (!FALSE)
    
    #define BAD (-1)
    #define GOOD (0)
    
    #ifndef SUCCESS
    #define SUCCESS  0
    #endif
    #ifndef FAILURE
    #define FAILURE  -1
    #endif
    

    That is verbatim, copy+paste from the file, no lines inbetween were removed.

    System functions that return a "boolean", alternate between the TRUE/FALSE syntax and the GOOD/BAD one (even related functions are different). Note that they are incompatible and that a standard C boolean comparison (if(x)) will evaluate BAD as TRUE and GOOD as FALSE.

    To make it worse, some of the declarations resolve simply to "int" so that's no help (there is also a "bool" typedef that is unsigned that some others use) but I found on at least 1 occasion where the documentation specified the wrong convention.

     

    Here's another example;

    typedef int ssize_t;		/* This is mostly a size_t, that can also be -1 */
    

    Mostly? Ok, so that's only a little suspect, this would be a superb WTF if a function that returned a "ssize_t" gave negative values other than -1, right?... well, guess what?

    Same file, a little later;

    #define SF_UNABLE_TO_WRITE_IN_FILE		-0x01 
    #define SF_UNABLE_TO_READ_FILE			-0x02
    #define SF_BUFFER_NOT_BIG_ENOUGH		-0x03
    #define SF_MESSAGE_NOT_FOUND			-0x04
    #define SF_MESSAGE_NOT_OWNER			-0x05
    

    Care to guess what type these values are provided as?



    P.S What library/project is that from? URL?


Log in to reply