<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 26, 2022 at 2:49 PM Rob Landley <<a href="mailto:rob@landley.net">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 3/26/22 11:59 AM, Moritz C. Weber wrote:<br>
> Dear all,<br>
> I wrote the simple patch to enable wget for POST requests with an --post-data option.<br>
> Sadly, patched wget and git HEAD segfault on my current build machine.<br>
<br>
I did some cleanups to wget recently (after talking to you about it), and<br>
applied your patch on top of those.<br>
<br>
You got the argument order backwards in GLOBALS, the arguments from right to<br>
left in the NEWTOY() string populate the structure from top to bottom. I need to<br>
finish the tutorial video on that...<br></blockquote><div><br></div><div>or reverse them in the code? this always gets me too. is it easier to just fix it (by having a max that we subtract from?) so struct fields can be in the "obvious" order, rather than add more explanations of why it's "backwards" (for non-RTL language natives at least)?</div><div><br></div><div>it would be a disruptive diff to say the least, but better sooner than later?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I admit it's a little nonobvious. The bits go from right to left,<br>
NEWTOY(blah, "abcd", FLAGS) means d is (1<<0), c is (1<<1), b is (1<<2) and so<br>
on. So "blah -a -b -d" would set toys.optflags to binary 1101.<br>
<br>
GLOBALS() is essentially treated as a union of the command's type structure, and<br>
an array of long that gets populated in packed bit order. So "a#b*cd:" would be:<br>
<br>
GLOBALS(<br>
  char *d;<br>
  struct arg_list *b;<br>
  long a;<br>
)<br>
<br>
Since "-c" doesn't take an argument, it doesn't consume a slot.<br>
<br>
Linux is LP64 so sizeof(pointer) == sizeof(long), and register sized entries in<br>
structures have no packing between or before them, so -d arg goes in TT.d, -b<br>
arg goes in TT.b, and -a arg goes it TT.a.<br>
<br>
Note that when they're the same type I collate the declarations, and C<br>
declarations go "left to right then top to bottom". So:<br>
<br>
NEWTOY(blah, "a#b:cd:", FLAGS)<br>
GLOBALS(<br>
  char *d, *b;<br>
  long a;<br>
)<br>
<br>
NEWTOY(blah, "a#b#cd:", FLAGS)<br>
GLOBALS(<br>
  char *d;<br>
  long b, a;<br>
)<br>
<br>
(By convention I put a blank line between variables populated by arguments and<br>
globals that are just used by the code to store stuff. The latter are always<br>
initialized to zero.)<br>
<br>
> So, testing can and will be improved, but I mainly send this patch to learn<br>
> the process. And I think the change is trivial, if I got everything right.<br>
> <br>
> I am not used to write C and I am still learning the toybox infrastructure.<br>
> So I would be happy to get feedback what to do better.<br>
<br>
sizeof(pointer) gives you the size of the pointer variable (4 bytes on 32 bit<br>
systems, 8 bytes on 64 bit systems). You wanted strlen(pointer). I fixed it up<br>
already.<br>
<br>
The other thing is an issue that was already there in wget (and part of the<br>
reason it's still in pending): toybuf is a 4k buffer but there's no limit on the<br>
length of the input arguments, so sprintf(toybuf, xxx) can run off the end of<br>
the buffer. I need to add length checks. (Working on it...)<br>
<br>
> The long-term purpose for this patch would be to refactor the wget logic into<br>
> the library, so that I could reuse GET and POST in a git toy, which I plan.<br>
<br>
That is good to do. The cleanups I'm doing should help towards that.<br>
<br>
Thanks,<br>
<br>
Rob<br>
_______________________________________________<br>
Toybox mailing list<br>
<a href="mailto:Toybox@lists.landley.net" target="_blank">Toybox@lists.landley.net</a><br>
<a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" rel="noreferrer" target="_blank">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</blockquote></div></div>