On 25/04/2021 22:26, Jason Molenda wrote:
I was looking at lldb-platform and I noticed I implemented the A packet in it, 
and I was worried I might have the same radix error as lldb in there, but this 
code I wrote made me laugh:

     const char *p = pkt.c_str() + 1;   // skip the 'A'
     std::vector<std::string> packet_contents = 
get_fields_from_delimited_string (p, ',');
     std::vector<std::string> inferior_arguments;
     std::string executable_filename;

     if (packet_contents.size() % 3 != 0)
     {
         log_error ("A packet received with fields that are not a multiple of 3:  
%s\n", pkt.c_str());
     }

     unsigned long tuples = packet_contents.size() / 3;
     for (int i = 0; i < tuples; i++)
     {
         std::string length_of_argument_str = packet_contents[i * 3];
         std::string argument_number_str = packet_contents[(i * 3) + 1];
         std::string argument = decode_asciihex (packet_contents[(i * 3) + 
2].c_str());

         int len_of_argument;
         if (ascii_to_i (length_of_argument_str, 16, len_of_argument) == false)
             log_error ("Unable to parse length-of-argument field of A packet: %s in 
full packet %s\n",
                        length_of_argument_str.c_str(), pkt.c_str());

         int argument_number;
         if (ascii_to_i (argument_number_str, 16, argument_number) == false)
             log_error ("Unable to parse argument-number field of A packet: %s in 
full packet %s\n",
                        argument_number_str.c_str(), pkt.c_str());

         if (argument_number == 0)
         {
             executable_filename = argument;
         }
         inferior_arguments.push_back (argument);
     }


These A packet fields give you the name of the binary and the arguments to pass 
on the cmdline.  My guess is at some point in the past the arguments were not 
asciihex encoded, so you genuinely needed to know the length of each argument.  
But now, of course, and you could write a perfectly fine client that mostly 
ignores argnum and arglen altogether.

That's quite clever, actually. I like it. :)



I wrote a fix for the A packet for debugserver using a 'a-packet-base16' 
feature in qSupported to activate it, and tested it by hand, works correctly.  
If we're all agreed that this is how we'll request/indicate these protocol 
fixes, I can put up a phab etc and get this started.


I think that's fine, though possible changing the servers to just ignore the length fields, like you did above, might be even better, as then they will work fine regardless of which client they are talking to. They still should advertise their non-brokenness so that the client can form the right packet, but this will be just a formality to satisfy protocol purists (or pickier servers), and not make a functional difference.

pl
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Reply via email to