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

Submitter:  Joel Cunningham <jcunningham>
Submitted:  Fri 11 Aug 2017 03:29:01 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  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>
Group administrator
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>
Group administrator
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>
Group administrator
Sat 23 Dec 2017 07:34:53 PM UTC, comment #4: 

Right

Simon Goldschmidt <goldsimon>
Group administrator
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>
Group Member
Fri 11 Aug 2017 06:28:39 PM UTC, comment #1: 

Looks good.

Simon Goldschmidt <goldsimon>
Group administrator
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>
Group Member

 

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

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)
  •  

    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 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.13-3230.
    Corresponding source code