Reply
Thread Tools
Posts: 2,802 | Thanked: 4,491 times | Joined on Nov 2007
#101
Originally Posted by iDont View Post
I'm currently holding the release off so others can comment on the patch.
Since no one else wants to comment, here goes. Sorry if it sounds like I'm picking on you, I'm actually very happy to see someone else looking at this problem from a different perspective :-)

Code:
copy_file(tmphistory, storedhistory, 11); /* 11 = force copy & preserve permissions */
11 is actually FILEUTILS_FORCE | FILEUTILS_DEREFERENCE | FILEUTILS_PRESERVE_STATUS, one of which is lost in translation above ;-) Why not use symbolic constants here? The code would be self-documenting, and also protected against future flag changes.

Code:
if (access(CONFIG_ASH_HIST_BUFFER_PATH, R_OK == -1))
	bb_make_directory(xasprintf("%s", CONFIG_ASH_HIST_BUFFER_PATH), 01777,
If /tmp isn't there you've got bigger problems, I would just log something to stderr and proceed with no history. Besides, a) a non-root shell wouldn't be able to create it anyway, b) the return value of bb_make_directory() is ignored, and c) the mode might be inappropriate if CONFIG_ASH_HIST_BUFFER_PATH points to a different place.

Code:
tmphistory = concat_path_file(CONFIG_ASH_HIST_BUFFER_PATH, 
	xasprintf(".ash_hist_%s", user));
That's a predictable filename in a world-writable dir again. To illustrate, imagine that one of your "mates" grabs your N900 when you're not looking (and before you've had a chance to run a root shell after a reboot) and types something like "ln -s /etc/passwd /tmp/.ash_hist_root". Now imagine what happens next time you start a root shell.

Code:
close(open(storedhistory, O_WRONLY | O_CREAT, 0644));
I missed this last time, why not just use creat(2)? Come to think of it, there's no need to create storedhistory at this point, you could just creat() tmphistory directly and skip the copy - one write less :-)

Code:
if (access(tmphistory, R_OK | W_OK) == -1)
	copy_file(storedhistory, tmphistory, 11); /* 11 = force copy & preserve permissions */
This will have race conditions no matter what checks you do here, so best to just use a private per-user path as in the earlier version. Also, same comment on 11 as above.
 

The Following 4 Users Say Thank You to lma For This Useful Post:
Posts: 2,802 | Thanked: 4,491 times | Joined on Nov 2007
#102
Originally Posted by vi_ View Post
You can wget the page and look at it in vim to confirm that it does in fact contain the weather report text.
Ouch, that page is a mess, line-ending-wise :-(. Most of it is dos/windows-style (CRLF), but the paragraphs you're interested in are old-MacOS-style (CR), and of course you're trying to get sensible returns in a system that expects LF which doesn't help :-/

You could add a "tr '\r' '\n'" to the pipeline to translate those into LFs, or "tr -d '\r'" to simply remove them.
 

The Following 5 Users Say Thank You to lma For This Useful Post:
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#103
I've just pushed some commits to busybox-power's GIT repository, including one to include and enable a new version of the shell history patch (albeit a bit later than intended). I'm pretty confident about this one, with the patch being modified several times already. As said before, if no issues are found with the new version of the patch, it will be included in the next busybox-power release. You can find the patch here. Corrections etc. are highly appreciated.

For those who haven't read previous pages of this thread: with the mentioned patch, BusyBox' history issues with using multiple shell instances are gone.

Originally Posted by lma View Post
Sorry if it sounds like I'm picking on you, I'm actually very happy to see someone else looking at this problem from a different perspective :-)
Don't worry, it didn't sound so to me . Again, thanks for the review, much appreciated.

Last edited by iDont; 2011-07-27 at 14:06.
 

The Following 3 Users Say Thank You to iDont For This Useful Post:
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#104
I've got some great news: busybox-power is now available in Maemo's extras repository! A huge thanks goes out to all the people who have voted my package up

In other news, busybox-power 1.18.5power3 will be released in a week or so. As it will have some nice features over 1.18.5power2, I won't push the latter into extras-testing, but push the upcoming new busybox-power version directly into there.

Update: busybox-power 1.18.5power3 has just been pushed to extras-devel. I especially want to thank lma for providing me multiple times with feedback. I'll promote this new version to -testing tomorrow. If I may quote myself:
Originally Posted by iDont View Post
Lastly, I've promoted busybox-power to extras-testing [..] Please subject the package to some serious testing, so it might arrive in Maemo's extras repository sometime in the future
All votes are highly appreciated. Thanks in advance.

Last edited by iDont; 2011-07-31 at 13:27.
 

The Following 3 Users Say Thank You to iDont For This Useful Post:
Posts: 2,225 | Thanked: 3,822 times | Joined on Jun 2010 @ Florida
#105
Wooo @ Extras-Devel and -testing for the new BusyBox power. What with the glorious shell history tweeks and all. I was actually surprised when opening a second terminal session on my N900 earlier today, to find the commands I had JUST typed in the first one, all in my history already. So... right.... Lack of words....
 

The Following 3 Users Say Thank You to Mentalist Traceur For This Useful Post:
Posts: 2,225 | Thanked: 3,822 times | Joined on Jun 2010 @ Florida
#106
Question to anyone using my /sbin/preinit script modification for getting an optional command-line at boot: do you have any issues with it ever since this last version of busybox-power came out?

Mine randomly started segfaulting on me, right at the moment the "sh" line is supposed to run, and I only noticed it a few days ago, but the only thing that changed since I last saw it working was busybox-power and my fiddling around with the R&D mode CAL area (I'm writing a small command-line program in C to change the R&D mode flags from on-device, based on qwerty12's gui program for the same purpose - but I've tried resetting the R&D mode flags using qwerty's program [which I know doesn't mess up the R&D flags, since I used his tool all this time], and rebooting, and I still see a segmentation fault in the console right at the moment the "sh" command runs in my /sbin/preinit modification.)

So, anyone getting the same behavior, or is it just me?
 

The Following 2 Users Say Thank You to Mentalist Traceur For This Useful Post:
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#107
Mentalist Traceur: I'm experiencing those segfaults as well. It most likely does that because either root's directory is still RO or /tmp isn't available in that stage of the boot process yet.
I wanted to reply to your PM earlier, but I hadn't had access to the internet prior to today. I'm currently outide the country, so I can't dig into the issue yet. I'll be back home in a little over a week or so.
 

The Following 2 Users Say Thank You to iDont For This Useful Post:
Posts: 2,225 | Thanked: 3,822 times | Joined on Jun 2010 @ Florida
#108
Originally Posted by iDont View Post
Mentalist Traceur: I'm experiencing those segfaults as well. It most likely does that because either root's directory is still RO or /tmp isn't available in that stage of the boot process yet.
I wanted to reply to your PM earlier, but I hadn't had access to the internet prior to today. I'm currently outide the country, so I can't dig into the issue yet. I'll be back home in a little over a week or so.
Sorry I just noticed this reply today. Being in America I am spoiled to expect free internet in hotels, but I know that's not the norm in a lot of places, so I appreciate you going out of your way to reply while on your trip.

The /tmp directory is mounted just before my boot-shell code executes in /sbin/preinit if you use my exact code. I don't think root file system is read-only either, because I've edited /sbin/preinit when inside the shell launched by /sbin/preinit... (Though I think the boot process errors out right after the file save, probably something to do with it trying to execute the file's code but then having the file changed right under it.) Whether or not everything in the root file system is mounted though, or any parts haven't been 'made' yet (if there's anything that's in /tmp or anywhere else that's needed for the current busybox-power but that gets dynamically made/remade/updated later in the boot process).

So, in other words, way out of my league, but there is a "mount tmpfs [...] /tmp" line right before my code in /sbin/preinit. Either way, your latest reply to me by PM today is promising because it suggests this problem won't be relevant as soon as you implement history buffering inside busybox itself instead of the original 'use a temporary file' method.
 

The Following 3 Users Say Thank You to Mentalist Traceur For This Useful Post:
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#109
BusyBox 1.19 has been released a little over a week ago. It contains about 9 months of much appreciated hard work by the BusyBox developers & other contributors. It sure makes a great update

Busybox-power has been updated accordingly: 1.19.0power1 just got pushed to Maemo's extras-devel. I'm letting it settle there for a few days before promoting it to extras-testing because of the size of the update.

Head over to http://busybox.net/ if you want to find out more about the changes that went into BusyBox 1.19. Changes specific to busybox-power include fixing the issue discovered by Mentalist Traceur in the posts above, busybox being split in busybox and busybox_root (see bug #700), and the activation of a few new applets.
I'd like to point out one particularly interesting new feature of BusyBox 1.19: Reverse history search. Whilst already common in other shells, searching your shell history by pressing Ctrl-R is now available in BusyBox' shells too.

Enjoy the new release!
 

The Following 6 Users Say Thank You to iDont For This Useful Post:
Posts: 7 | Thanked: 3 times | Joined on Jul 2011
#110
Hello.

Does v1.19.0power1 lack ipv6 support?
 

The Following 3 Users Say Thank You to Socim For This Useful Post:
Reply

Thread Tools

 
Forum Jump


All times are GMT. The time now is 23:03.