[BusyBox] Re: Some more busybox issues...

[Mark Whitley]

On Thu, Mar 15, 2001 at 07:40:35PM +0100, Andreas Neuhaus wrote:
> hello all,
> 
> while others were, i saw that my fix suggestions were not included into cvs
> yet. do i have to submit an official bugreport via the web page or is it
> usually ok to post suggestions to busybox at busybox.net?

No we were just busy doing other stuff and didn't respond to your email right
away. Sorry about that.

I would reccommend that you use the bug system to report bugs simply because
it helps us to keep track of them better. That, and reports in the
bug-tracking system are harder to ignore.

> sorry, i don't want to hurry you, i was just thinking those things are
> important enough to fix before releasing 0.50.

You have indeed found some important bugs and we want 0.50 to be solid. Thanks
for the testing.

> 1) sh.c fails compiling with uClibc (called object is not a function). fix
> is in one of my previous mails

Good catch. I was able to reproduce this bug reliably on my own box when
compiling busybox against uClibc. I applied the patch from your previous
email, tested compiling again, saw that it worked, and committed the change to
CVS. Please check out the latest source and see if it works for you.

> 2) linuxrc never works because applets.h uses BB_LINUXRC which is never
> defined. fix is in one of my previous mails

Erik recently checked in a linuxrc-related fix but I'm not sure if it was
yours. Will you please check out the latest source and see if the problem is
still there?

> here are some more things i found. i'm using the latest busybox from cvs
> linked against the latest uClibc from cvs on a 2.4.2 linux kernel (devfs
> mounted, but no devfsd):
> 
> 3) lash export behaves strange when setting a var that was defined before.
> the old env entry isn't removed. e.g.: when PATH=/bin and doing export
> PATH=/sbin:$PATH there are 2 PATH entries afterwards: one says PATH=/bin,
> the other PATH=/sbin:/bin. doing this again results in 3 PATH entries.

This is usingthe latest cvs source and busybox compiled against uClibc:


$ ./busybox sh


BusyBox v0.50pre (2001.03.15-19:13+0000) Built-in shell (lash)
Enter 'help' for a list of built-in commands.

/home/markw/devel/busybox $ echo $PATH
/home/markw/bin:/usr/local/bin:/usr/local/games:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin/X11:/usr/games
/home/markw/devel/busybox $ export PATH=newpath
/home/markw/devel/busybox $ echo $PATH
sh: echo: No such file or directory
/home/markw/devel/busybox $ /bin/echo $PATH
newpath


Sorry, I'm not seeing it. Can you post a testcase that can show me how to
reproduce this bug? A bunch of changes were recently made to env var support
in busybox shell. Are you using the latest source?


> 4) ps does not work for me. the sccanf calls in parse_proc_status in ps.c
> always return an empty string. i discovered this when searching for the
> reason why kernel processes are listed as "[]" instead of "[swapper]" and
> process status is missing completely.

Good catch. I am able to duplicate this on my system. I'll submit this one to
bug tracking.

> 5) argv[0]="init"; in init.c does not work for some reason. this results in
> init staying as /linuxrc in the process list. this furtheron results in
> failure of reboot, halt and poweroff because they are unable to find the
> "init" process.

Erik is most familiar with init and is probably the best person to look at
this. Just for kicks, can you try changing the line to argv[0]=strdup("init");
and see if that fixes it?

> 6) kernel 2.4.2 allows it to run /sbin/init from linuxrc (see
> Documentation/initrd.txt). should busybox/init.c really instist on being pid
> 1 when running as init?
> 
> 7) at several places (e.g. init.c) elements in linked lists are deleted
> while iterating through it. this can be potentially dangerous because the
> next pointer is used from an already freed element. e.g. in init.c line 957:
>         for (a = initActionList; a; a = a->nextPtr) {
>                 if (a->action == ONCE) {
>                         run(a->process, a->console, FALSE);
>                         delete_initAction(a);
>                 }
>         }
> when delete_initAction(a); returnes, a was usually free()d. but the for loop
> uses a->nextPtr to get the next element. i didn't run into problems with
> this, but it seems like this could become a problem(?).

The  delete_initAction function accounts for the removed item. The function is
below with the line that handles the pointer juggling labeled as such:

	static void delete_initAction(initAction * action)
	{
		initAction *a, *b = NULL;

		for (a = initActionList; a; b = a, a = a->nextPtr) {
			if (a == action) {
				if (b == NULL) {
					initActionList = a->nextPtr;
				} else {
					b->nextPtr = a->nextPtr; /* <-- Pointer juggling handled here */
				}
				free(a);
				break;
			}
		}
	}

So, I don't think this particular case is a problem. If you find any examples
where linked-lists are not being managed correctly, patches are welcome.


Hope this helps.


Mark Whitley
markw at lineo.com