I now have a new version against 45a that I apply to -46 and it can now be a
module as I fixed from atoi to simple_strtol
which is exported.
I also added a umusb_urb_linkctx and spin_locks to protect the list so that
it does not race on my computer (badly).
I agree more clean up is necessary.
From: "Jeff Dike" <jdike@...>
To: "James McMechan" <James_McMechan@...>
Sent: Monday, July 29, 2002 4:03 PM
Subject: Re: [uml-devel] usb-uml-2.4.18-45
> James_McMechan@... said:
> > Though the indenting may be even worse it is now standard.
> It's better, I suppose. Still needs a bunch of work.
Yes I agree, but I am first attempting to get it to work then to clean up
> Why does it have its own double-linked list implementation
no clue, worse it needed locking see attached usb.fix.bz2 for large hammer
(spin_lockirqsave) locking which gets rid of the major race condition that
was killing my testing.
Also the cleanup function did not remove the timer which blows up "x"
jiffies after module remove.
> Most of the comments are useless. They're no better than
> i++; /* increment i */
> It looks like you undid most of the sTuDLyCapS, which is a good thing,
> but now it looks like a bunch more underscores are needed.
Ok, like umusb_urb_unlink_ctx/link_ctx until we start using the kernel
> The userspace/kernelspace separation is poor. In umusb_user.c, I see
> #define KERN_INFO "<6>"
I put that in due to laziness I just did not want the flood of user side
messages without a printk priority and I just crammed that in to allow for
global message twiddling.
> spinlock_t umusb_busses_lock = SPIN_LOCK_UNLOCKED;
This is also a mess I am still working at making it work enough for me to
grok the usb structure
I have not understood enough to start major shuffles of the architecture of
the usb driver itself.
I have been concentrating on making it mostly work without hard crashes on
my machine and getting to be able to have it as a module.
> umusb_pollfds doesn't appear to be used for anything.
There were gobs of unused functions unreachable code and unused vars.
I have been dropping every thing I could find that is unused just so I can
study what is in use.
> Then there's this charming snippet:
> unsigned char path[PATH_MAX + 1];
among other things...
> where 'grep PATH_MAX /usr/include/*/*.h' yields
> /usr/include/linux/limits.h:#define PATH_MAX 4095
> nicely blowing out the kernel stack.
> What's the matter with using sizeof(dev->hubdev->portinfo.port) here?
> memset(dev->hubdev->portinfo.port, 0, 127);
Nothing but time to get to it.
> How is this superior to !strcmp(driver.driver, "hub")?
> return ((driver.driver == 'h') &&
> (driver.driver == 'u') &&
> (driver.driver == 'b') && (driver.driver == '\0'));
It shouldn't be but that may hit more of libc which has already caused
> More magic numbers:
> unsigned char desc;
> while ((ret = read(udev->fd, desc, 64)))
> The memsets of newly allocated structures are pointless. All they do is
> obscure bugs.
As in killing the slab poisoning effect?
> This was after a fairly quick look at the code. It needs a good thorough
More gardening in progress...