On Fri, 2008-10-17 at 15:43 -0700, Linus Torvalds wrote:
> 
> On Fri, 17 Oct 2008, Dave Airlie wrote:
> > 
> > Please pull the 'drm-next' branch from
> > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git 
> > drm-next
> 
> Grr.
> 
> This whole merge series has been full of people sending me UNTESTED CRAP.
> 
> So what's the excuse _this_ time for adding all these stupid warnings to 
> my build log? Did nobody test this?
> 
>   drivers/gpu/drm/drm_proc.c: In function ‘drm_gem_one_name_info’:
>   drivers/gpu/drm/drm_proc.c:525: warning: format ‘%d’ expects type ‘int’, 
> but argument 3 has type ‘size_t’
>   drivers/gpu/drm/drm_proc.c:533: warning: format ‘%9d’ expects type ‘int’, 
> but argument 4 has type ‘size_t’
>   drivers/gpu/drm/i915/i915_gem.c: In function ‘i915_gem_gtt_pwrite’:
>   drivers/gpu/drm/i915/i915_gem.c:184: warning: unused variable ‘vaddr_atomic’
> 
> and I wonder how many other warnings got added that I never even noticed 
> because I don't build them?
> 
> And yes, it's not just warnings. One of thse is horribly bad code:
> 
>       nid->len += sprintf(&nid->buf[nid->len],
>                             "%6d%9d%8d%9d\n",
>                             obj->name, obj->size,
>                             atomic_read(&obj->handlecount.refcount),
>                             atomic_read(&obj->refcount.refcount));
> 
> where it's just wrong to use the field width as a separator. Because if 
> the counts are big enough, the separator suddenly goes away!
> 
> So that format string should be
> 
>       "%6d %8zd %7d %8d\n"
> 
> instead. Which  gives the same output when you don't overflow, and doesn't 
> have the overflow bug when you do.
> 
> As to that "vaddr_atomic" thing, the warning would have been avoided if 
> you had just cleanly split up the optimistic fast case.
> 
> IOW, write cleaner code, and the warning just goes away on its own. 
> Something like the appended. UNTESTED!

Yeah, none of us are on x86-64, so we missed those warnings in testing.
The change looks pretty good except for s/return 1/return 0/.  We wanted
to pull the i_wish_it_was_ioremap_atomic() hack out so that we could use
it for relocation updates as well (which aren't copy_from_user), and
I've started writing that patch.  Expect something pushed at you soon.

-- 
Eric Anholt
[EMAIL PROTECTED]                         [EMAIL PROTECTED]


Attachment: signature.asc
Description: This is a digitally signed message part

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to