buglwIP - A Lightweight TCP/IP stack - Bugs: bug #35874, reserved identifier violation

 
 

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

bug #35874: reserved identifier violation

Submitted by:  Markus Elfring <elfring>
Submitted on:  Sat 17 Mar 2012 02:40:19 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: Change RequestStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: 1.5.0
lwIP version: 1.4.0

(Jump to the original submission Jump to the original submission)

Thu 22 May 2014 08:30:43 AM UTC, comment #11:

Thanks for your source code improvement.

Markus Elfring <elfring>
Tue 20 May 2014 10:32:24 AM UTC, comment #10:

Fixed again (changed H_ to _H).

Everyone satisfied? :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 20 May 2014 10:23:57 AM UTC, comment #9:

> The remaining usage of double underscores does not fit to C++ naming conventions


Yeah, I though so, too. Just was too lazy to do the 2nd renaming...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 19 May 2014 09:10:35 PM UTC, comment #8:

Comment #7:

> ... I used "LWIP_HDR_xxx_H__" to create the include guards ...

http://git.savannah.gnu.org/cgit/lwip.git/commit/?id=dbd125c714f27254991cf85fa6a5a3626d2650fa

The remaining usage of double underscores does not fit to C++ naming conventions.
http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier#answer-228797

Markus Elfring <elfring>
Mon 19 May 2014 08:26:32 PM UTC, comment #7:

> A little late here


Not at all too late (not until it's released, anyway).

However, the intention of the bug/change was to not collide with reserved identifiers. And using three underscores is not in the spirit of the reserved identifiers, as I read it...

Anyway:
a) I used "LWIP_HDR_xxx_H__" to create the include guards and
b) if it really collides with something, it won't compile, so people can either change their files or come back here... :-)

Sorry if this sounds harsh, it's not meant to. I just don't see a clean way out of this when lwIP is supposed to be a library but is effectively somewhere between user code (no underscores) and standard code (one or more underscores...)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 19 May 2014 08:12:51 PM UTC, comment #6:

A little late here but without underscores there's a risk of colliding with user's names with this change that could impact existing code. I myself use LWIP_xxxx for my own LWIP defines.

Since you are reserving these for the lwIP code base I would have suggested 3 underscores then to keep the change completely transparent.

Bill Auerbach <billauerbach>
Mon 19 May 2014 07:59:35 PM UTC, comment #5:

Done. Hope I caught them all...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 27 Mar 2012 05:51:39 PM UTC, comment #4:

Being like that, I'd target this for 1.5.0 (not that high prio to me, or did you have actual problems with the identifiers as they are now?).

Working on it, I'd love to restructure all defines to begin with "LWIP_" to prevent possible future portability problems.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 18 Mar 2012 09:15:38 PM UTC, comment #3:

Re Simon's comment about C++ and C99 standards, it is worth pointing out that identifiers beginning with double underscore or underscore followed by an uppercase letter are also reserved by the ANSI/ISO C-1990 standard (see section 7.1.3), which is the basis of most embedded C compilers. This is not a new restriction in C99.

David Empson <dempson>
Sun 18 Mar 2012 04:53:44 PM UTC, comment #2:

Would you like to integrate the appended adjustments into your source code repository?

(file #25394, file #25395)

Markus Elfring <elfring>
Sun 18 Mar 2012 12:33:48 PM UTC, comment #1:

I don't really know if the cited standards (C++, C99) apply to us as lwIP does not use C++ and most embedded compilers no nothing about C99, however, I guess it wouldn't hurt to change these identifiers...

Would you mind creating a patch against git master?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 17 Mar 2012 02:40:19 PM UTC, original submission:

I suggest to try the search pattern "\<_(?:(?:_(.*))|([A-Z]+))" on source files.
Some places will be found where names begin with two underscores or an underscore and an uppercase letter.

Examples:

This usage does not fit to the expected naming conventions of the C/C++ language standard.
I recommend to adjust your selection for unique names.

Markus Elfring <elfring>

 

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by billauerbach (Posted a comment)
  • -unavailable- added by dempson (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by elfring (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 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 20 May 2014 10:32:24 AM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Tue 20 May 2014 10:23:57 AM UTCgoldsimonStatusFixed=>In Progress
      Open/ClosedClosed=>Open
    Mon 19 May 2014 07:59:35 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Tue 27 Mar 2012 05:51:39 PM UTCgoldsimonPlanned Release=>1.5.0
    Sun 18 Mar 2012 04:53:44 PM UTCelfringAttached File-=>Added 0001-Bug-35874-Make-a-configuration-symbol-standard-compl.patch, #25394
      Attached File-=>Added 0001-Bug-35874-Make-include-guards-standard-compliant.patch, #25395

    Back to the top


    Powered by Savane 3.1-cleanup