patchlwIP - A Lightweight TCP/IP stack - Patches: patch #6765, Supporting new line characters in...

 
 

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

patch #6765: Supporting new line characters in inet_aton()

Submitted by:  Tai-hwa Liang <atliang>
Submitted on:  Wed 04 Mar 2009 03:39:51 AM UTC  
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  Simon Goldschmidt <goldsimon> Open/Closed:  Closed
Planned Release:  1.3.1

( Jump to the original submission)

Fri 24 Apr 2009 12:15:05 PM UTC, comment #12: 

Ok, check in

Frédéric Bernon <fbernon>
Project Member
Fri 24 Apr 2009 10:42:54 AM UTC, comment #11: 

Oups, really sorry, I miss that in post #6 :((

I will check in that in the day if you don't do it before.

Frédéric Bernon <fbernon>
Project Member
Fri 24 Apr 2009 10:39:58 AM UTC, comment #10: 

> Can I check in the change I propose in comment #7


As it seems to be the same like what I posted in comment #6 (and tested, but not yet checked in), I think you can and this should fix it.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 24 Apr 2009 08:55:13 AM UTC, comment #9: 

Simon, are you agree with my last comments ? Can I check in the change I propose in comment #7

Frédéric Bernon <fbernon>
Project Member
Thu 23 Apr 2009 07:37:19 PM UTC, comment #8: 

With the change I propose, this code :

  char* dnsname[]={
   "127.0.0.1",
   "127.0.0.1 ",
   "127.0.0.1\n",
   "127.0.0.1\r",
   "abc.de",
   "3com.com",
   "3com.com ",
   "3com.com \n",
   "3com.com\n",
   "3com.com\r",
   "3com.com\r\n",
   "3com.com\r\n",
   "3com.com\n\r"};
  int i;
  for( i=0; i<(sizeof(dnsname)/sizeof(dnsname[0])); i++) {
    printf("[%i]\n%s\n0x%08lX\n", i, dnsname[i], inet_addr(dnsname[i]));
  }

give this output :

[0]
127.0.0.1
0x0100007F
[1]
127.0.0.1
0x0100007F
[2]
127.0.0.1

0x0100007F
[3]
127.0.0.1
0x0100007F
[4]
abc.de
0xFFFFFFFF
[5]
3com.com
0xFFFFFFFF
[6]
3com.com
0xFFFFFFFF
[7]
3com.com

0xFFFFFFFF
[8]
3com.com

0xFFFFFFFF
[9]
3com.com
0xFFFFFFFF
[10]
3com.com

0xFFFFFFFF
[11]
3com.com

0xFFFFFFFF
[12]
3com.com

0xFFFFFFFF

With your last change, the output is :

[0]
127.0.0.1
0x0100007F
[1]
127.0.0.1
0x0100007F
[2]
127.0.0.1

0x0100007F
[3]
127.0.0.1
0x0100007F
[4]
abc.de
0xFFFFFFFF
[5]
3com.com
0x03000000
[6]
3com.com
0x03000000
[7]
3com.com

0x03000000
[8]
3com.com

0x03000000
[9]
3com.com
0x03000000
[10]
3com.com

0x03000000
[11]
3com.com

0x03000000
[12]
3com.com

0x03000000

Frédéric Bernon <fbernon>
Project Member
Thu 23 Apr 2009 07:22:59 PM UTC, comment #7: 

I test again, but inet_addr("3com.com") return 0x3 (and in my point of view, we should got 0x00). It's due to the fact we leave the loop when we reach the first 'c', and in your change, isprint('c') is true, so, don't go to the next line (return 0).

To my point of view, something like this should be better :

if (c != '\0' && !isspace(c))

Frédéric Bernon <fbernon>
Project Member
Thu 23 Apr 2009 04:33:45 PM UTC, comment #6: 

Sorry, seems like I messed that up. I converted

if (c != '\0' && (!isprint(c) || !isspace(c)))

to

if (c != '\0' && !isprint(c) && !isspace(c))

since the first version returned always 'true' if c != 0.

if (c != '\0' && !isspace(c))

seems to give the correct results, tested for:

"abc.de"
"127.0.0.1"
"127.0.0.1 "
"127.0.0.1\n"
"127.0.0.1\r"
"3com.com"
"3com.com "
"3com.com \n"
"3com.com\n"
"3com.com\r"
"3com.com\r\n"
"3com.com\r\n"
"3com.com\n\r"

Anyone something against that?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 23 Apr 2009 03:46:19 PM UTC, comment #5: 

yes, I can ! :)

Frédéric Bernon <fbernon>
Project Member
Thu 23 Apr 2009 03:45:14 PM UTC, comment #4: 

Could you try reverting this change and see if it fixes the problem you're seeing Frédéric?

Kieran Mansley <kieranm>
Project Member
Thu 23 Apr 2009 03:41:26 PM UTC, comment #3: 

I'm not sure if there is a real relation, but now, the win32 port resolve inet_aton("3com.com") as 0.0.0.3 (I'm pretty sure that it was working - returns 0 -  there is a long time ago)

Frédéric Bernon <fbernon>
Project Member
Wed 15 Apr 2009 07:06:51 PM UTC, comment #2: 

I guess the actual bug here is fixed like this:

-  if (c != '\0' && (!isprint(c) || !isspace(c)))
+  if (c != '\0' && !isprint(c) && !isspace(c))

I'll check it in, please reopen if you feel otherwise.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 09 Apr 2009 03:25:30 PM UTC, comment #1: 

Anyone got any objections to this?  The proposed patch does allow all characters with ascii code < 0x20 to be accepted, not just \n and \r.  It's not clear to me what the original line was trying to achieve:

  /*
   * Check for trailing characters.
   */
  if (c != '\0' && (!isprint(c) || !isspace(c)))
    return (0);

Kieran Mansley <kieranm>
Project Member
Wed 04 Mar 2009 03:39:51 AM UTC, original submission:  

Following calls return the same on *BSD, Linux and Solaris:

  inet_addr("192.168.1.1")
  inet_addr("192.168.1.1\n")
  inet_addr("192.168.1.1\r\n")

Without the attached patch, the trailing new line characters will cause current inet_addr() in lwIP to return 0xffffffff.

Tai-hwa Liang <atliang>

 

Attached Files
file #17577:  inet.c.patch added by atliang (875B - isprint -> isascii in inet_aton())

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Updated the item)
  • -email is unavailable- added by atliang (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-04-24 fbernon Open/ClosedOpen => Closed
    2009-04-23 fbernon Open/ClosedClosed => Open
    2009-04-15 goldsimon StatusNone => Done
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2009-04-03 kieranm Planned ReleaseNone => 1.3.1
    2009-03-04 atliang Attached File- => Added inet.c.patch, #17577

    Back to the top


    Powered by Savane 3.5