Open johnco3 opened 9 years ago
Hi @johnco3
Nice that you are trying this lib on windows. I wrote it in 4 hours mainly and did just test it on linux.
I'm interested in bringing it to windows too, just didn't take the time for the moment. I think apart from compiler differencies like this one, there is one additional thing that will be needed : handling path with .native() and not with generic_string, because for the moment I handle path in the Unix style.
But this is a small detail. I'll give it a try on my windows vm to tell you what it might be. Looks like I indeed have some strange ambiguations with phoenix.
It might also be the boost version, I compile mainly this code with Boost 1.55.0 for now. But I'll try with the last boost version on windows. I won't be there until Tuesday, so I might not fix it before middle of next week.
Keep me informed if you get any improvements.
I just started to get it to work, amazing job for 4 hours work.
The fix to getting the server to compile was to prefix the rule with boost::spirit::qi::rule in the request_grammar struct and to prefix the rule with boost::spirit::karma::rule in the response_grammar otherwise the compiler complains about ambiguities.
template<typename Iterator>
struct request_grammar : grammar <Iterator, possible_request()> {
/** Pases a request packet */
* boost::spirit::qi::*rule<Iterator, possible_request()> request;
/** Parses an ascii name */
boost::spirit::qi::rule<Iterator, std::string()> name;
/** Parses mode (netascii, octet, mail) */
boost::spirit::qi::rule<Iterator, mode()> mode_name;
...
template<typename OutputIterator>
struct response_grammar : grammar <OutputIterator, possible_response()>
{ /* Generates a response packet _/ boost::spirit::karma_::rule<OutputIterator, possible_response()> response; boost::spirit::karma::rule<OutputIterator, data_response()> data; boost::spirit::karma::rule<OutputIterator, error_response()> error; boost::spirit::karma::rule<OutputIterator, option_ack()> oack;
I'm completely new to the karma/qi grammar checker/generators - I get a bunch of ugly warnings as shown below - but the tftp server links and runs and I am in the middle of debugging it to see if it can serve a jpg file as a test. In my case I am going to need a tftp client but I am not sure exactly how much I will need to tweak the code to do that. Also note that the cmake settings for windows are completely wrong, In particular the generated -Werror flag makes the compiler fail - I ended up just making a simple visual studio project adding the missing things by hand.
John
1>C:\temp\tftp\tftp\tftp/server.hpp(285): warning C4101: 'err' :
unreferenced local variable
1>C:\Program Files (x86)\Microsoft Visual Studio
12.0\VC\include\xutility(2132): warning C4996: 'std::_Copy_impl': Function
call with parameters that may be unsafe - this call relies on the caller to
check that the passed values are correct. To disable this warning, use
-D_SCL_SECURE_NO_WARNINGS. See documentation on how to use Visual C++
'Checked Iterators'
1> C:\Program Files (x86)\Microsoft Visual Studio
12.0\VC\include\xutility(2113) : see declaration of 'std::_Copy_impl'
1>
C:\main\extlibs\boost_1_58_0\boost/spirit/home/karma/detail/output_iterator.hpp(230)
: see reference to function template instantiation '_OutIt
std::copystd::_String_const_iterator<std::_String_val<std::_Simple_types
Now that I am debugging the server on windows, I think I may have spotted a problem or 2. It looks like you only send a single final block (your particular use case may have been only for a file < a single block) as you set the exception on the fstream for the and you do not open it in binary mode. So far the use of generic string is not causing any problems.
Anyway it looks like its fixed now. I attach the updated server.hpp - apologies for the changes to the formatting, but I passed the headers through a beautifier just to make the code a bit clearer to understand and made adjusted for some inconsistencies (it's not my intention to change your formatting style). I also adjusted the signatures of the lambda callbacks to match the asio documentation const& etc, I also added a fix the following what looks like a copy/paste error.
I see that upload file support is present, I'll see if I can understand the code sufficiently to tweak that to work as it would be great to have a fully functional open source implementation of an asio tftp server - coupled with a corresponding client it would be amazing and then I could use it in an avionics application that I am developing.
oack.name("oack") instead of oack.name("error")
John
On Thu, May 7, 2015 at 1:37 PM, Damien Buhl notifications@github.com wrote:
Hi @johnco3 https://github.com/johnco3
Nice that you are trying this lib on windows. I wrote it in 4 hours mainly and did just test it on linux.
I'm interested in bringing it to windows too, just didn't take the time for the moment. I think apart from compiler differencies like this one, there is one additional thing that will be needed : handling path with .native() and not with generic_string, because for the moment I handle path in the Unix style.
But this is a small detail. I'll give it a try on my windows vm to tell you what it might be. Looks like I indeed have some strange ambiguations with phoenix.
It might also be the boost version, I compile mainly this code with Boost 1.55.0 for now. But I'll try with the last boost version on windows. I won't be there until Tuesday, so I might not fix it before middle of next week.
Keep me informed if you get any improvements.
— Reply to this email directly or view it on GitHub https://github.com/daminetreg/lib-tftp-server/issues/2#issuecomment-99953262 .
Sorry I forgot to attach the tweaked files
John
On Thu, May 7, 2015 at 1:37 PM, Damien Buhl notifications@github.com wrote:
Hi @johnco3 https://github.com/johnco3
Nice that you are trying this lib on windows. I wrote it in 4 hours mainly and did just test it on linux.
I'm interested in bringing it to windows too, just didn't take the time for the moment. I think apart from compiler differencies like this one, there is one additional thing that will be needed : handling path with .native() and not with generic_string, because for the moment I handle path in the Unix style.
But this is a small detail. I'll give it a try on my windows vm to tell you what it might be. Looks like I indeed have some strange ambiguations with phoenix.
It might also be the boost version, I compile mainly this code with Boost 1.55.0 for now. But I'll try with the last boost version on windows. I won't be there until Tuesday, so I might not fix it before middle of next week.
Keep me informed if you get any improvements.
— Reply to this email directly or view it on GitHub https://github.com/daminetreg/lib-tftp-server/issues/2#issuecomment-99953262 .
Now that I am debugging the server on windows, I think I may have spotted a problem or 2. It looks like you only send a single final block (your particular use case may have been only for a file < a single block) as you set the exception on the fstream for the and you do not open it in binary mode. So far the use of generic string is not causing any problems.
No the server is serving everyday for 5 developers files biggers than 100 mbs from their own pcs to barebox client devices, which have a really bad implementation of tftp, which made me tweak it to be more resilient to client who does whatever they want.
As I told in some commits the spaghetti for ASIO isn't that cool. :smile: Nice that you are going on them.
However if you want me to merge things, you'll have to stick to my formatting, just to ease review and avoid having completely crazy diffs of whitespaces changes only.
Regarding the CMakeLists, it just needs adding an If condition following the built platform in the CMakeLists.txt.
The warnings you get are not a good thing, possibly you get errors because of these. There shouldn't be any, on linux it built perfectly cleanly. Next tuesday I'll give it a try.
If you open a fork, I could pick the things you already did.
Damien, thanks, I'm new to github (not to git) so bear with me while I get familiar with the forking strategy. I'll check in the code once I get the tftp put functionality more or less working. So far I have the tftp put succesfully negotiation the options and I'm working on a "save_current_file" implementation modeled from your serve_current_file. I've kind of got it started now. One thing I will need from this tftp server is the ability to support several concurrent puts and gets, not quite sure how I will accomplish this yet - but its early days and I know at the very least I will need a sort of session identifier (called a TID in tftp terms) to disambiguate the sessions. I've done this before in a previous project (actually the one I am porting to use asio). From an asio perspective, I'm not sure if we will need multiple sockets to accomplish this - one that listens to the tftp service port 69 (actually in my case 59) and others that are created on demand when RRQ and WRQ requests arrive.
On Fri, May 8, 2015 at 4:47 AM, Damien Buhl notifications@github.com wrote:
Now that I am debugging the server on windows, I think I may have spotted a problem or 2. It looks like you only send a single final block (your particular use case may have been only for a file < a single block) as you set the exception on the fstream for the and you do not open it in binary mode. So far the use of generic string is not causing any problems.
No the server is serving everyday for 5 developers files biggers than 100 mbs from their own pcs to barebox client devices, which have a really bad implementation of tftp, which made me tweak it to be more resilient to client who does whatever they want.
As I told in some commits the spaghetti for ASIO isn't that cool. [image: :smile:] Nice that you are going on them.
However if you want me to merge things, you'll have to stick to my formatting, just to ease review and avoid having completely crazy diffs of whitespaces changes only.
Regarding the CMakeLists, it just needs adding an If condition following the built platform in the CMakeLists.txt.
The warnings you get are not a good thing, possibly you get errors because of these. There shouldn't be any, on linux it built perfectly cleanly. Next tuesday I'll give it a try.
If you open a fork, I could pick the things you already did.
— Reply to this email directly or view it on GitHub https://github.com/daminetreg/lib-tftp-server/issues/2#issuecomment-100158312 .
Damien, I managed to get the tftp put code working - but I'm not sure how the whole spirit qi and karma things work and I was hoping for a bit of help to format a data_request. Right now the request_grammar contains the following 3 types of request and I think that it would be useful if we could also include a data_request, but I cannot seem to get that to work.
request_grammar() : request_grammar::base_type(request) {
request =
(big_word(opcode::read_request) > name > lit('\0') >
mode_name > lit('\0') > options)[_val = construct
I was thinking that a data_request would be something along the following
lines
(big_word(opcode::data) > big_word > big_word)[_val =
construct
BOOST_FUSION_DEFINE_STRUCT( (tftp)(detail), data_request, (uint16_t, blocknum) (std::string, data) )
but this is not compiling and I cannot figure out why.
Meanwhile I had to include a call to ntohs which explicitly gets the block in the expected endian format.
// wait for the data block here
socket_.async_receive_from(
boost::asio::buffer(data_, max_length), sender_endpoint_,
[this, blocknum](const error_code& ec, size_t bytes_recvd) {
if (!ec && bytes_recvd >= 4) {
// make sure that the block is the expected one
auto packetBlockNum =
ntohs(reinterpret_cast<uint16t*>(data)[1]); if (packetBlockNum == blocknum) { // send ACK tftp::detail::acknowledgment response(blocknum); auto ackBuffer = generate_response(response); std::cout << "saving block[" << blocknum << "]" << std::endl;
On Fri, May 8, 2015 at 4:47 AM, Damien Buhl notifications@github.com wrote:
Now that I am debugging the server on windows, I think I may have spotted a problem or 2. It looks like you only send a single final block (your particular use case may have been only for a file < a single block) as you set the exception on the fstream for the and you do not open it in binary mode. So far the use of generic string is not causing any problems.
No the server is serving everyday for 5 developers files biggers than 100 mbs from their own pcs to barebox client devices, which have a really bad implementation of tftp, which made me tweak it to be more resilient to client who does whatever they want.
As I told in some commits the spaghetti for ASIO isn't that cool. [image: :smile:] Nice that you are going on them.
However if you want me to merge things, you'll have to stick to my formatting, just to ease review and avoid having completely crazy diffs of whitespaces changes only.
Regarding the CMakeLists, it just needs adding an If condition following the built platform in the CMakeLists.txt.
The warnings you get are not a good thing, possibly you get errors because of these. There shouldn't be any, on linux it built perfectly cleanly. Next tuesday I'll give it a try.
If you open a fork, I could pick the things you already did.
— Reply to this email directly or view it on GitHub https://github.com/daminetreg/lib-tftp-server/issues/2#issuecomment-100158312 .
I was trying to get this code to compile on windows with the latest version of boost 1.58 and as I'm very new to the spirit, qi, karma grammar logic, I am not quite able to figure out what is wrong. Basically I get the attached rule ambiguity errors and changing the stand alone rule to boost::spirit::qi::rule does not work around the compilation issue.
1>------ Build started: Project: tftp-server, Configuration: Debug Win32 ------ 1> main.cpp 1> Please define _WIN32_WINNT or _WIN32_WINDOWS appropriately. For example: 1> - add -D_WIN32_WINNT=0x0501 to the compiler command line; or 1> - add _WIN32_WINNT=0x0501 to your project's Preprocessor Definitions. 1> Assuming _WIN32_WINNT=0x0501 (i.e. Windows XP target). 1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(152): error C2872: 'rule' : ambiguous symbol 1> could be 'boost::phoenix::rule' 1> or 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/qi/nonterminal/rule.hpp(88) : boost::spirit::qi::rule' 1> C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(192) : see reference to class template instantiation 'tftp::detail::parser::request_grammar' being compiled
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(152): error C2143: syntax error : missing ';' before '<'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(152): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(152): error C2238: unexpected token(s) preceding ';'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(157): error C2872: 'rule' : ambiguous symbol
1> could be 'boost::phoenix::rule'
1> or 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/qi/nonterminal/rule.hpp(88) : boost::spirit::qi::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(157): error C2143: syntax error : missing ';' before '<'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(157): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(157): error C2238: unexpected token(s) preceding ';'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(162): error C2872: 'rule' : ambiguous symbol
1> could be 'boost::phoenix::rule'
1> or 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/qi/nonterminal/rule.hpp(88) : boost::spirit::qi::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(162): error C2143: syntax error : missing ';' before '<'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(162): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(162): error C2238: unexpected token(s) preceding ';'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(167): error C2872: 'rule' : ambiguous symbol
1> could be 'boost::phoenix::rule'
1> or 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/qi/nonterminal/rule.hpp(88) : boost::spirit::qi::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(167): error C2143: syntax error : missing ';' before '<'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(167): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(167): error C2238: unexpected token(s) preceding ';'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(212): error C2872: 'rule' : ambiguous symbol
1> could be 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/karma/nonterminal/rule.hpp(89) : boost::spirit::karma::rule'
1> or 'boost::phoenix::rule'
1> C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(236) : see reference to class template instantiation 'tftp::detail::generator::response_grammar' being compiled
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(214): error C2872: 'rule' : ambiguous symbol
1> could be 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/karma/nonterminal/rule.hpp(89) : boost::spirit::karma::rule'
1> or 'boost::phoenix::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(215): error C2872: 'rule' : ambiguous symbol
1> could be 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/karma/nonterminal/rule.hpp(89) : boost::spirit::karma::rule'
1> or 'boost::phoenix::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/detail/tftp_packet_grammar.hpp(216): error C2872: 'rule' : ambiguous symbol
1> could be 'C:\main\extlibs\boost_1_58_0\boost/spirit/home/karma/nonterminal/rule.hpp(89) : boost::spirit::karma::rule'
1> or 'boost::phoenix::rule'
1>C:\temp\lib-tftp-server-master\src\tftp/server.hpp(246): warning C4244: 'argument' : conversion from 'std::streamsize' to 'unsigned int', possible loss of data
========== Build: 0 succeeded, 1 failed, 1 up-to-date, 0 skipped ==========