From: Nelson, E. - 2 <eri...@ba...> - 2010-01-21 16:57:43
|
In boost/network/uri/http/uri_concept.hpp line 22, I get a warning about conversion from a 32-bit to 16-bit integer 'initializing' : conversion from 'boost::uint32_t' to 'boost::uint16_t', possible loss of data Here's the offending line uint16_t port_ = port(uri); I'm not sure if it matters (probably doesn't) but if the truncation is as designed we might consider making it explicit with a cast. Thanks Erik |
From: Jeroen H. <vex...@gm...> - 2010-01-21 17:14:56
Attachments:
port.patch
|
Hi, On Thu, Jan 21, 2010 at 17:57, Nelson, Erik - 2 <eri...@ba...> wrote: > In boost/network/uri/http/uri_concept.hpp line 22, I get a warning about > conversion from a 32-bit to 16-bit integer > > 'initializing' : conversion from 'boost::uint32_t' to 'boost::uint16_t', > possible loss of data > > Here's the offending line > > uint16_t port_ = port(uri); > > I'm not sure if it matters (probably doesn't) but if the truncation is > as designed we might consider making it explicit with a cast. > > Thanks > > Erik > It seems I missed two uint32_t's when moving to the uint16_t type for ports, I've attached a trivial patch which should fix this. The attached patch also updates the documentation and tests. Jeroen |
From: Dean M. B. <mik...@gm...> - 2010-01-21 17:18:19
|
On Fri, Jan 22, 2010 at 1:14 AM, Jeroen Habraken <vex...@gm...> wrote: > > It seems I missed two uint32_t's when moving to the uint16_t type for > ports, I've attached a trivial patch which should fix this. The > attached patch also updates the documentation and tests. > Are these patches also on your fork? Those would be good to have merged into the 0.5-devel line soon. BTW, are your changes ready to merge into 0.5-devel so I can test locally? -- Dean Michael Berris cplusplus-soup.com | twitter.com/deanberris linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com |
From: Jeroen H. <vex...@gm...> - 2010-01-21 17:26:27
Attachments:
port.patch
|
On Thu, Jan 21, 2010 at 18:17, Dean Michael Berris <mik...@gm...> wrote: > On Fri, Jan 22, 2010 at 1:14 AM, Jeroen Habraken <vex...@gm...> wrote: >> >> It seems I missed two uint32_t's when moving to the uint16_t type for >> ports, I've attached a trivial patch which should fix this. The >> attached patch also updates the documentation and tests. >> > > Are these patches also on your fork? Those would be good to have > merged into the 0.5-devel line soon. No, they aren't, and I was about to ask. It might be easiest here if I create a couple of issues and attach the patches there. > BTW, are your changes ready to merge into 0.5-devel so I can test locally? Not really, there is still quite a bit of work to be done as I've described in the URI thread, and major functionality like the IP parsing is missing. As I have exams at the moment it's hard to tell when I have time to work on this. The current code in my fork should compile and run though, and I'd like your opinion on the way I implemented the derived HTTP class in my fork. Also, my code breaks some of the current code, it seems mostly from the transition from protocol() to scheme(), so those need to be fixed. > -- > Dean Michael Berris > cplusplus-soup.com | twitter.com/deanberris > linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com > Sorry, attached the wrong patch, this one will actually fix the documentation and tests. Jeroen |
From: Dean M. B. <mik...@gm...> - 2010-01-21 17:40:19
|
On Fri, Jan 22, 2010 at 1:25 AM, Jeroen Habraken <vex...@gm...> wrote: > On Thu, Jan 21, 2010 at 18:17, Dean Michael Berris > <mik...@gm...> wrote: >> >> Are these patches also on your fork? Those would be good to have >> merged into the 0.5-devel line soon. > > No, they aren't, and I was about to ask. It might be easiest here if I > create a couple of issues and attach the patches there. > Definitely. >> BTW, are your changes ready to merge into 0.5-devel so I can test locally? > > Not really, there is still quite a bit of work to be done as I've > described in the URI thread, and major functionality like the IP > parsing is missing. As I have exams at the moment it's hard to tell > when I have time to work on this. The current code in my fork should > compile and run though, and I'd like your opinion on the way I > implemented the derived HTTP class in my fork. > Alright, sure let me take a look and let you know soon enough. :) > Also, my code breaks some of the current code, it seems mostly from > the transition from protocol() to scheme(), so those need to be fixed. > Cool, no worries there. Let me know when you've done these changes on your branch too and once the tests pass we should be good to go for a merge. > > Sorry, attached the wrong patch, this one will actually fix the > documentation and tests. > No problem, thanks for these! -- Dean Michael Berris cplusplus-soup.com | twitter.com/deanberris linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com |
From: Jeroen H. <vex...@gm...> - 2010-01-21 19:02:13
|
On Thu, Jan 21, 2010 at 18:39, Dean Michael Berris <mik...@gm...> wrote: > On Fri, Jan 22, 2010 at 1:25 AM, Jeroen Habraken <vex...@gm...> wrote: >> On Thu, Jan 21, 2010 at 18:17, Dean Michael Berris >> <mik...@gm...> wrote: >>> >>> Are these patches also on your fork? Those would be good to have >>> merged into the 0.5-devel line soon. >> >> No, they aren't, and I was about to ask. It might be easiest here if I >> create a couple of issues and attach the patches there. >> > > Definitely. Added a couple of issues with patches in github, the one that is remaining is the discussion on `and` etcetera. >>> BTW, are your changes ready to merge into 0.5-devel so I can test locally? >> >> Not really, there is still quite a bit of work to be done as I've >> described in the URI thread, and major functionality like the IP >> parsing is missing. As I have exams at the moment it's hard to tell >> when I have time to work on this. The current code in my fork should >> compile and run though, and I'd like your opinion on the way I >> implemented the derived HTTP class in my fork. >> > > Alright, sure let me take a look and let you know soon enough. :) > >> Also, my code breaks some of the current code, it seems mostly from >> the transition from protocol() to scheme(), so those need to be fixed. >> > > Cool, no worries there. Let me know when you've done these changes on > your branch too and once the tests pass we should be good to go for a > merge. > >> >> Sorry, attached the wrong patch, this one will actually fix the >> documentation and tests. >> > > No problem, thanks for these! > > -- > Dean Michael Berris > cplusplus-soup.com | twitter.com/deanberris > linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com > Jeroen |
From: Dean M. B. <mik...@gm...> - 2010-01-21 19:06:54
|
On Fri, Jan 22, 2010 at 2:54 AM, Jeroen Habraken <vex...@gm...> wrote: > On Thu, Jan 21, 2010 at 18:39, Dean Michael Berris > <mik...@gm...> wrote: > > Added a couple of issues with patches in github, the one that is > remaining is the discussion on `and` etcetera. > Thanks for those, I see them. I'll apply and test locally before pushing to 0.5-devel. :) -- Dean Michael Berris cplusplus-soup.com | twitter.com/deanberris linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com |