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