Messages in this thread | | | Date | Sun, 30 Mar 2003 09:41:54 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: Update direct-rendering to current DRI CVS tree. |
| |
On Sun, 30 Mar 2003, Dave Jones wrote: > > This bit seems to be backing out a memleak fix..
Well, yes and no. I looked at the code, and decided that the memleak fix was hottibly bogus.
Look at how the allocations are done inside the loop: they are INSIDE A LOOP.
Look at what happens with the memory leak fix if the loop fails after having done <n> iterations: it will only unregister the last (failing) iteration, but it will then free stuff that the earlier (successful) iterations still depend on.
> > @@ -41,7 +42,7 @@ > > > > /* malloc/free without the overhead of DRM(alloc) */ > > #define DRM_MALLOC(x) kmalloc(x, GFP_KERNEL) > > -#define DRM_FREE(x) kfree(x) > > +#define DRM_FREE(x,size) kfree(x) > > eww. wtf is this for ? Some cross-OS compataiblity gunk ?
I assume it's for *BSD.
> > #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,4,2) > > #define down_write down > > #define up_write up > > #if can go, like it did in other parts of the patch.
Only if _you_ are willing to sync the DRM tree.
Since I'm only doing the DRM syncs occasionally, I want to keep unnecessary differences between the kernel and DRM to a minimum. Which means that I do remove gunk that I find stupid and ugly (I've removed #ifdef's in the code before), but not stuff that looks like reasonable compatibility stuff.
> I would read through the other 10 billion lines, but I lost > enthusiasm..
Ehh.. I _did_ go through the other lines, and was actually involved with one of the more spread-out changes (ie suggesting the fixed locking context thing). As far as I can tell the patch is good. I'd still like more eyes on the result, but the eyes should look a bit more closely..
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |