buglwIP - A Lightweight TCP/IP stack - Bugs: bug #24132, Cross-dependency between ip_addr.h...

 
 

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

bug #24132: Cross-dependency between ip_addr.h and inet.h

Submitter:  Luca Ceresoli <lucaceresoli>
Submitted:  Mon 25 Aug 2008 10:51:27 AM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Compiler Warning Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.3.0

Jump to the original submission

Wed 11 Feb 2009 08:22:34 PM UTC, comment #9: 


> I expect warnings on ip_addr.h: it uses ntohl() and companion functions, which are defined in inet.h.


Ooops, missed that. It did not generate warnings, but that's because all files that use the macro include inet.h...

I've fixed that.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 10 Feb 2009 09:39:13 PM UTC, comment #8: 

Thanks Simon.

I can't test it right now, but I expect warnings on ip_addr.h: it uses ntohl() and companion functions, which are defined in inet.h.

> ip_addr.h still contains the IN_CLASS* macros, which look like
> inet.h from the prefix, but at least for me, that's OK.

They are BSD compatibility defines, so inet.h would be the "right" place. Anyway they never hurt me where they are.

Luca Ceresoli <lucaceresoli>
Tue 10 Feb 2009 08:47:17 PM UTC, comment #7: 

OK, I've checked in the changes requested in comment #4, but removed the dependency from ip_addr.h to inet.h. ip_addr.h still contains the IN_CLASS* macros, which look like inet.h from the prefix, but at least for me, that's OK.

Luca, awaiting your comment.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 30 Sep 2008 01:53:48 PM UTC, comment #6: 

Reopening per request.

Jonathan Larmour <jifl>
Group Member
Tue 30 Sep 2008 10:32:00 AM UTC, comment #5: 

Ok, it's marked as closed (but it shouldn't), and it's by no means critical, but in case anybody forgot about this bug, here comes a reminder...

Luca Ceresoli <lucaceresoli>
Thu 28 Aug 2008 03:18:55 PM UTC, comment #4: 

Here's a patch that is a lot cleaner:

  • moves INADDR_* defines to inet.h where they are more appropriate
  • does not add (improper) includes in inet.c
  • takes the opportunity to define INADDR_ANY and _BROADCAST



(file #16379)

Luca Ceresoli <lucaceresoli>
Wed 27 Aug 2008 03:49:36 PM UTC, comment #3: 


> moved declaration of struct in_addr from ip_addr.h to inet.h

I'm ok with this change, but one point is still there: ip4_addr1() and similar macros in ip_addr.h use ntohl(), which is in inet.h.

Since thanks to your change now inet.h does not need ip_addr.h anymore, the attached patch tries to go one step forward.
It simply swaps the dependancy: ip_addr.h includes inet.h and not the vice versa.
I'm not 100% satisfied because it also silently makes inet.c (not .h) dependent on ip_addr.h, but it might still be ok since ip_addr.h is mostly a bunch of macros.

Er, the patch leaves to be fixed files (if any) that include inet.h but use ip_addr.h.


(file #16367)

Luca Ceresoli <lucaceresoli>
Tue 26 Aug 2008 07:47:23 PM UTC, comment #2: 

-> moved declaration of struct in_addr from ip_addr.h to inet.h

Simon Goldschmidt <goldsimon>
Group administrator
Tue 26 Aug 2008 07:42:57 PM UTC, comment #1: 

Wouldn't it be more simple to move the declaration of struct in_addr to inet.h? Seems more logical to me as this is a struct that is used for the inet_*() functions only.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 25 Aug 2008 10:51:27 AM UTC, original submission:  

ip_addr.h defines:

> #define ip4_addr1(ipaddr) ((u16_t)(ntohl((ipaddr)->addr) >> 24) & 0xff)

and other macros which use ntohl().

This generates compiler warnings: ip_addr.h should include inet.h, where ntohl() is.

But inet.h already includes ip_addr.h (for using struct in_addr)!

Idea 1: mode ntohl() and related to a new file :-(

Idea 2: reimplement ip4_addr1() without using ntohl :-S such as:
#define ip4_addr1(ipaddr) ((u16_t)((char*)((ipaddr)->addr)[0]) & 0xff)
or similar

Luca Ceresoli <lucaceresoli>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #16379:  inet-ip-addr2.patch added by lucaceresoli (1KiB - text/x-patch)
file #16367:  inet-ip-addr.patch added by lucaceresoli (1002B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by lucaceresoli (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-02-11 goldsimon Open/ClosedOpen Closed
    2009-02-10 goldsimon StatusIn Progress Fixed
    2008-09-30 jifl StatusFixed In Progress
        Open/ClosedClosed Open
    2008-08-28 lucaceresoli Attached File- Added inet-ip-addr2.patch, #16379
    2008-08-27 lucaceresoli Attached File- Added inet-ip-addr.patch, #16367
    2008-08-26 goldsimon CategoryNone IPv4
        StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code