bugAVR Downloader/UploaDEr - Bugs: bug #35265, Findings reported by cppcheck

 
 

You are not allowed to post comments on this tracker with your current authentification level.

bug #35265: Findings reported by cppcheck

Submitted by:  Rene Liebscher <rliebscher>
Submitted on:  Sun 08 Jan 2012 05:18:52 PM UTC  
 
Category: NoneSeverity: 2 - Minor
Priority: 5 - NormalItem Group: None
Status: In ProgressPrivacy: Public
Assigned to: NoneOriginator Name: Rene Liebscher
Open/Closed: Open

Fri 13 Jan 2012 10:22:04 PM UTC, comment #5:

I made a first patch which replaces all usleep by microsleep and adds a file util.c which realizes microsleep either by nanosleep, usleep or by a Windows specififc function. (For the last it moved the code from ppiwin.c to util.c)

MinGW has only usleep but no nanosleep (at least configure does not find it) so I left the usleep variant in the code.

(file #24815)

Rene Liebscher <rliebscher>
Project Member
Tue 10 Jan 2012 07:18:18 AM UTC, comment #4:

> I agree about the new implementation file.


With one exception though: avrdude.c is just too generic a
name. That name should probably be the name of the main
(CLI) file (rather than main.c).

My suggestion is to name it util.c or such.

avrdude.h should simply combine the exported interfaces
(by libavrdude.a) from all the existing header files. The
current header file structure is a nuisance for anyone who
wants to use the library. If there are library-internal
interfaces that are not supposed to be exported (so they
could be easily changed between library versions), they could
go into a second header file that is only used to build the
library itself, but not going to be installed even in case
we once install the library itself.

Joerg Wunsch <joerg_wunsch>
Project Administrator
Mon 09 Jan 2012 10:27:47 PM UTC, comment #3:

OK, thanks, with the commandline, I can reproduce the
messages now.

> 'Our' usleep is the old posix usleep


You're right. While I don't think any of the actual systems
are going to remove it in the near future, it certainly makes
sense to replace it by nanosleep().

Given that nanosleep() is quoted (in FreeBSD's manpage) to
originate in Posix.1 (IEEE Std 1003.1b-1993), are there any
actual systems around we want to be portable to (various
Linuxes, *BSD, Solaris, MacOS X) which don't have it?

I agree about the new implementation file.

Joerg Wunsch <joerg_wunsch>
Project Administrator
Mon 09 Jan 2012 08:11:12 PM UTC, comment #2:

command line was 'cppcheck --enable=all .'

cppcheck version is 'Cppcheck 1.49'

'Our' usleep is the old posix usleep (except on Windows were we have a replacement in ppiwin.c).

But: POSIX.1-2001 declares this function obsolete; use nanosleep(2) instead. POSIX.1-2008 removes the specification of usleep(). (see https://www.kernel.org/doc/man-pages/online/pages/man3/usleep.3.html)

So we better replace all occurrences of usleep by an own wrapper microsleep. To use nanosleep we have check it in the configure file.

I would put this wrapper in a new C-file (avrdude.c?, as we have an avrdude.h defining a usleep for Windows.) I also would move the implementations of usleep and gettimeofday from ppiwin.c to this new file as they are used not only by the parallel port programmers.

Rene Liebscher <rliebscher>
Project Member
Sun 08 Jan 2012 10:37:44 PM UTC, comment #1:

Could you please post you commandline?

> (style) Found obsolete function 'usleep'


Maybe we should rename our usleep() into microsleep().

Joerg Wunsch <joerg_wunsch>
Project Administrator
Sun 08 Jan 2012 05:18:52 PM UTC, original submission:

When running cppcheck the following findings are reported:

Also there are a lot of findings "(style) Found obsolete function 'usleep'. It is recommended that new applications use the 'nanosleep' or 'setitimer' function", which I filtered out.

Some errors were already removed in svn revision 1035.

Rene Liebscher <rliebscher>
Project Member

 

Attached Files
file #24815:  util.c added by rliebscher (4kB - text/x-csrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by joerg_wunsch (Posted a comment)
  • -unavailable- added by rliebscher (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 13 Jan 2012 10:22:04 PM UTCrliebscherAttached File-=>Added util.c, #24815
      StatusNone=>In Progress

    Back to the top


    Powered by Savane 3.1-cleanup