On Sunday 24 October 2010 11:02, Rob Landley wrote:
> On Saturday 23 October 2010 20:20:01 Denys Vlasenko wrote:
> > I wanted to post a very similar rant about uclibc. But did not. Ranting
> > usually doesn't help. Doing something about things you want to see improved
> > is the way forward.
>
> For uClibc, I tended to mail cakes to people. It seems to work, for some
> reason.
>
> Alas at this point, it's hard to consider the uClibc project anything other
> than a zombie. NTPL for uClibc has been in development for _five_years_ and
> still hasn't shipped. It took the original NPTL developers what, six months
> to get a working implementation in glibc? And they had to do the kernel-side
> infrastructure while they were at it. The uClibc web page hasn't been
> updated
> in over 6 months...
>
> I know, I should download the git snapshots and regression test and go engage
> with that project more... It's on the todo list.
So let's do it.
My problem with working on NTPL is that I dislike everything thread-related.
The only piece of software of the 1024 CPU machine which _has to be_ threaded
is the kernel, everything else is easier to parallelize on a task basis:
don't waste time degeloping, say, 1024-CPU parallelized gzip - instead,
run thousands of gzip copies!
It will be somewhat hard for me to work on NTPL in uclibc...
> > So why are you so grumpy? :D
>
> Eh, I'm usually grumpy. I yell at my own code all the time. (That's why I
> keep rewriting it so much.)
Well, this reduces the number of people who want to cooperate with you.
People are illogical. They better take criticism if it is stuff sugar-coated.
> Once upon a time I spent months each rewriting individual busybox apps, over
> and over, until I was pleased with the results. And umount was one of them,
> so it seemed safe to go in and make a specific change to busybox umount, to
> add
> a small and simple feature that should have been maybe a three line patch.
>
> Instead I got sidetracked into wondering why dietlibc #ifdefs and hurd
> patches
> were splattered all over the code since the last time I looked at it. "git
> show fbedacfc8caa1" is a huge mess replacing realpath(), which is a
> posix.1-2001 function.
realpath to a char buf[PATH_MAX] has a problem of requiring 4k large buffer.
In almost all cases, xmalloc_realpath() will be MUCH smaller. I liked
this patch mostly for this reason.
Maybe we need to have allocating version of getmntent too.
> We weren't even using the NULL extension we were doing
> our own malloc, and it got replaced with:
>
> + char *resolved_path = xmalloc_realpath(*argv);
> + if (resolved_path != NULL) {
> puts(resolved_path);
> + free(resolved_path);
>
> So we have an xmalloc function and we're checking the null return. Huh?
> Either it's badly named, or the caller is wasting bytes.
NULL here happens olny when realpath fails to "realpathize" its argument.
Say, xmalloc_realpath("/it/does/not/exist") is NULL.
> - safe_strncpy(path, m->dir, PATH_MAX);
> + path = xstrdup(m->dir);
>
> What was wrong with xstrdup() here?
The change replaces safe_strncpy with xstrdup, not the other way around.
> And what's "safe" about randomly
> truncating a string (which isn't necessary on Linux), this arbitrarily breaks
> umount on really long paths when it might otherwise work fine.
"safe" in safe_strncpy means "it will always be NUL-terminated" (unlike
strncpy).
>
> - realpath(zapit, path);
> - for (m = mtl; m; m = m->next)
> - if (!strcmp(path, m->dir) || !strcmp(path, m-
> >device))
> - break;
> + path = xmalloc_realpath(zapit);
> + if (path) {
> + for (m = mtl; m; m = m->next)
> + if (strcmp(path, m->dir) == 0 ||
> strcmp(path, m->device) == 0)
> + break;
> + }
>
> Posix 2008 says we can rely on realpath() doing a malloc with NULL now, so
> wrapping it seems like something we'd want to _undo_ now. As for the rest of
> it, the previous code was better. The new code is pointlessly verbose to
> perform the same function.
Readability. !strcmp() reads as "not strcmp" - ?
strcmp() != 0 reads as "not equal" - much closer to what it actually do.
I prefer more "spelled out" code on the source level exactly beause
it's easier to read. Your code is usually more densely written.
This is not a significant difference.
> Which means if I go in and start fiddling with umount, the first thing I'm
> likely do is break Hurd support. I haven't got a regression test
> environment,
> and I actively fail to care about that target, so I pretty much expect it.
Don't worry about it. It's up to hurd people (if any) to fix up.
> A
> significant part of my frustration is not knowing whether or not I'm
> _allowed_
> to clean out that crap to get us back to simple, or whether maintaining hurd
> support (which I can't test) is important.
>
> I don't understand what the requirements are for this code. Which means my
> approach would have to be "make a small localized change and don't change
> anything else because it's brittle black magic"... which takes all the fun
> out
> of a project like busybox.
If you want to change something but not sure whether it is ok,
post the patch(es) to the ml.
> > > When did simplicity stop being a goal?
> >
> > It did not. For example, sha1/256/512 and md5 hashes were recently
> > simplified.
>
> /me looks:
>
> Um...
>
> #if 0
> remaining = 128 - bufpos;
>
> /* Hash whole blocks */
> while (len >= remaining) {
> memcpy(ctx->wbuffer + bufpos, buffer, remaining);
> buffer = (const char *)buffer + remaining;
> len -= remaining;
> remaining = 128;
> bufpos = 0;
> sha512_process_block128(ctx);
> }
>
> /* Save last, partial blosk */
> memcpy(ctx->wbuffer + bufpos, buffer, len);
> #else
> yet more code...
>
> Uh-huh.
>
> Attached is the sha1sum.c I wrote for toybox a few years back. (Never did
> quite get around to cleaning it up, extending it to support the various other
> shaXXXsum sizes, or implementing md5sum. My todo list runneth over.) But
> what I was going for was _simple_, and I confirmed it produced the right
> output
> on all the data I threw at it. It's 185 lines including the actual applet
> implementation and help text and everything. The busybox one is 900 lines
> for
> the engine.
I saw it.
Wondered "how the hell Rod managed to squeeze sha1 into that few lines" :D
> Busybox has three or four different implementations of each piece of
> infrastructure in the sha1sum/md5sum stuff, depending on the "size vs speed"
> knob. I focused on doing it once in a way that was easiest to read and to
> understand.
>
> I.E. my implementation was simple first, worrying about small and fast
> second.
> The code you were referring to (current as of October 19th, git says) has a
> size vs speed config entry leading to a heavily redundant implementation, at
> the expense of simplicity, with rather a lot of what it's doing hidden in
> various macros.
This "size vs speed" thing im md5 is not mine. busybox-1.00 has it too.
As you can see, sha256/512 which I added don't have them.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
