On Fri, Jun 13, 2003 at 01:53:51AM +0200, Michel D?nzer wrote: > On Fri, 2003-06-13 at 00:31, [EMAIL PROTECTED] wrote: > > On Fri, Jun 13, 2003 at 12:00:12AM +0200, Michel D?nzer wrote: > > > > >It has no choice > > > but to take the data as it gets it from the client? > > > > Yes, but initially, the interesting piece of a header is the first char, and > > the kernel checks the first char (and then correctly skips the length of the > > union when it "knows" that the rest of the header doesn't apply to this > > particular type) hence it doesn't matter diddly squat what the value of the > > other bytes are. > > > > They are used on different headers, yes - but if there are packets output that > > aren't filling in all the data, passing zero instead of whatever the value > > should be is no more likely to work, is it? > > Unless some other code assumes the fields are cleared...
Well obviously, but if that code doesn't exist what have you fixed? Nothing. Or does the argument that some code may exist somewhere that works because the value isn't cleared, without referencing the code, an equally valid argument? :o) Let's assume that it does work now. If you add code in the future that needs the programmer to be aware of subtle semantics and side effects like AllocCmdBuf clears a few bytes, but not all why can't the same code and programmer simply know that the allocator doesn't clear data? If we're working on the theory that code will come to pass that is completely ignorant of existing chunks of DRI/DRM code, you've surely got to make the parts the programmer doesn't see work in a "least surprise" way? Personally I think (a) This allocator clears data or (b) this allocator doesn't - makes far more sense than "we clear a few bytes just in case" AllocCmdBuf is worth study, calling it can flush, lose context and dirty state. alloc_cmd_buf_possibly_lose_context_and_dirty_state_etc() to become alloc_cmd_buf_possibly_blah_blah_and_clear_the_first_few_bytes() ? Since my commit appears to have triggered the confusion that led to this - note what my not appreciating how AllocCmdBuf works led to :- (a) If you emit state by summing its size and AllocCmdBuf a big chunk it breaks - because AllocCmdBuf can dirty more state. This was what that patch "fixed" (b) If you emit state, as it does now, piecewise, if one of the calls to AllocCmdBuf flushes etc you need to emit all that state, that you just marked clean, again. Now (b) may only break when you've got 2 clients, dunno, but if you're looking at the patch I sent as a basis to improve the code. It probably makes a lot of sense to (a) not do that since I'm as clueless as you get and (b) look at how it works (or doesn't) rather than just clearing a word. Just my 2p's worth. > Well, if it's purely cosmetic, that's something even I might consider > worth a comment. :) True. Probably is a comment in the archives of dri-devel ;) but, to be frank, if you're going to change the code that sends data to the kernel without looking to see how the kernel uses the data you're going to get bitten anyway. -- Michael. ------------------------------------------------------- This SF.NET email is sponsored by: eBay Great deals on office technology -- on eBay now! Click here: http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5 _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
