buglwIP - A Lightweight TCP/IP stack - Bugs: bug #51729, lwip_itoa: fix converting 0

 
 

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

bug #51729: lwip_itoa: fix converting 0

Submitted by:  Joel Cunningham <jcunningham>
Submitted on:  Fri 11 Aug 2017 03:29:01 PM UTC  
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

( Jump to the original submission)

Tue 09 Jan 2018 01:56:50 PM UTC, comment #9: 

Well, since I don't push patch or bugfix on a regular basis, I didn't this weird behavior on this bugtracker. Thanks for advice.

Anyway, thank for applying it before I fix the style and rebase it to latest master.

Regards,
David

David GIRAULT <dgirault>
Mon 08 Jan 2018 08:39:26 PM UTC, comment #8: 

OK, fixed by applying your patch David. I had to modify it just a little...
Also fixed the unit test which did not detect modified *(buf-1)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 08 Jan 2018 08:09:20 PM UTC, comment #7: 

> Since I provided a patch earlier, any reason to not use it directly?


The main reason is that I did not see your patch. That's a limitation of this bug tracker: if you attach a patch file when writing a comment at the same time, you make the patch file appear as "file #xyz" in the discussion. If you just post the patch, it only gets listed down at the bottom (plus an email).

Added to that, know I look at it. The patch is not lwIP style and couldn't have been applied as is, anyway. I only see this now, though.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 08 Jan 2018 03:24:24 PM UTC, comment #6: 

New version is still buggy. It can write outside provided buffer if buffer isn't large enough:

char buf[4];
lwip_itoa(buf, 4, 1192);

This will write '1' at *(buf-1) !!!

Additionaly, why persisting to put "." in the buffer if it's too small at the end while, at same time, just an empty string is return at the beggining if to buffer_size < 2? This require caller to check both condition instead of just an empty string.

Since I provided a patch earlier, any reason to not use it directly?

David GIRAULT <dgirault>
Sat 23 Dec 2017 07:36:01 PM UTC, comment #5: 

Fixed and added unit tests for lwip_itoa.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 23 Dec 2017 07:34:53 PM UTC, comment #4: 

Right

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 19 Dec 2017 12:07:24 PM UTC, comment #3: 

lwip_itoa is still buggy! converting 0 is fixed (the bad way) but the following case still fail:

char buf[4];
lwip_itoa(buf, 4, 168);

It returns "68" in buf ! The same for 192. Returns "92" !

As it is use that way in mdns_build_reverse_v4_domain(), this result in BADLY encoded PTR answer in mDNS responder.

You may use an in-stack temporary buffer with enough space if required, but be able to convert numbers > 100 with a 4 char buffer.

Please re-open this bug.
Regards,
David

David GIRAULT <dgirault>
Fri 11 Aug 2017 06:40:36 PM UTC, comment #2: 

Patch committed in 9844049cb181d296840cf1a0e718bf960ca67ceb

Joel Cunningham <jcunningham>
Project Member
Fri 11 Aug 2017 06:28:39 PM UTC, comment #1: 

Looks good.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 11 Aug 2017 03:29:01 PM UTC, original submission:  

Found that 0 can't be converted when passed to lwip_itoa.  This was found via shell cmd idxtoname and win32 port.  "lo0" should
be returned for index 1 (instead "lo" was returned)

Attached patch adds a special case check for 0 before starting the conversion while loop.

Joel Cunningham <jcunningham>
Project Member

 

Attached Files
file #42681:  lwip_itoa.patch added by dgirault (2KiB - text/x-patch - Patch)
file #41480:  0001-lwip_itoa-fix-converting-0.patch added by jcunningham (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dgirault (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jcunningham (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 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-01-08 goldsimon StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2018-01-08 goldsimon StatusFixed => In Progress
        Open/ClosedClosed => Open
    2017-12-23 goldsimon StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2017-12-23 goldsimon StatusFixed => In Progress
        Assigned tojcunningham => goldsimon
        Open/ClosedClosed => Open
    2017-12-19 dgirault Attached File- => Added lwip_itoa.patch, #42681
    2017-08-11 jcunningham StatusNone => Fixed
        Open/ClosedOpen => Closed
    2017-08-11 jcunningham Attached File- => Added 0001-lwip_itoa-fix-converting-0.patch, #41480

    Back to the top


    Powered by Savane 3.5