#2 Check return codes everywhere

open
nobody
None
6
2007-11-21
2007-03-05
No

Some checks for return codes are missing.

Examples:
Would you like to add more error handling for return values from "malloc" like in the function "ProcessRequest" and from "select" in the function "main"?
http://downloads.sourceforge.net/tftp-server/tftpservermtV1.3.tar.gz

It looks a bit strange that the new operator and the class "std::string" is not used in a C++ source file.

Discussion

  • Achal Dhir

    Achal Dhir - 2007-03-07

    Logged In: YES
    user_id=1231444
    Originator: NO

    TFTP Server is not written in C++, it is C. Multithread version is pure C. However single port version uses STL, hence we need C++ compiler. C is much faster, sleep and more efficient in using resources.

    I did not face any malloc failures even in regression testing, but it is a good suggestion to check for pointer after malloc.

    Thanks You.

     
  • Markus Elfring

    Markus Elfring - 2007-03-07

    Logged In: YES
    user_id=572001
    Originator: YES

    The C programming language has got more potential for errors. The issue "unchecked function call" is very popular. There are more aspects to consider for efficiency, security and correctness.

    Are you going to rename any extension of your source files?

    Would the approach "http://cexcept.sourceforge.net/" be useful in your software?

     
  • Achal Dhir

    Achal Dhir - 2007-03-07
    • status: open --> closed
     
  • Markus Elfring

    Markus Elfring - 2007-11-21

    Logged In: YES
    user_id=572001
    Originator: YES

    The following example shows another programming error in your source file "tftpserver.cpp".
    "...
    pthread_create(&threadId, 0, processRequest, NULL);
    if (!threadId)
    ..."

    I also recommend to check if all locking operations were successfully executed. It is a bit more work needed to achieve a correct and safe thread pool.

     
  • Markus Elfring

    Markus Elfring - 2007-11-21
    • priority: 5 --> 6
    • status: closed --> open
     
  • Markus Elfring

    Markus Elfring - 2007-11-21

    Logged In: YES
    user_id=572001
    Originator: YES

    Another example for an open issue:
    "...
    if (ftell(req.file))
    fseek(req.file, 0, SEEK_SET);

    errno = 0;
    req.pkt[0]->opcode = htons(3);
    req.pkt[0]->block = htons(1);
    req.bytesRead[0] = fread(&req.pkt[0]->buffer, 1, req.blksize, req.file);

    if (errno)
    ..."

    - The error code "-1" is also "true" (non-zero).
    - It is more efficient to check a returned value directly than to use "errno".

     
  • Nobody/Anonymous

    Logged In: NO

    Thanks for your sharp observations. Although there is cpp extension to files, still is a C source code, not C++. calloc has been handled well by checking the returned pointer. What do you suggeset more. Can you please elaborate.

     
  • Markus Elfring

    Markus Elfring - 2008-02-19

    Logged In: YES
    user_id=572001
    Originator: YES

    I suggest to activate the SourceForge Subversion interface for your project. I guess that a better version repository will help to fix ignored error codes in the source files.