[Bug 17195] NamedPipe datagrams need to be _really_ datagrams

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=17195

--- Comment #127 from Adam Martinson <adam.r.martinson at gmail.com> 2013-01-03 21:04:20 CST ---
(In reply to comment #125)
> (In reply to comment #123)
> > You mean split patch 5 more?
> 
> Yes. If some app breaks after your commits, you'll get a regression test
> pointing to one of them. The smaller that patch is, the better for you. Apps
> tend to break on good commits too, so even if you're sure everything is OK,
> splitting is always a good idea.

I know that splitting is good xD  But that's the one where you stop getting a
pipe_instance handle and start getting a pipe_end handle for the server end. 
Those functions *must* change at the same time or stuff breaks.  Is there a way
to get around that that I'm not seeing?


(In reply to comment #124)
>  well, alexandre already asked for tests to be carried out, and you've already
>  explained that tests cannot be carried out unless the *entire* patch set is
>  applied and accepted, because it's so fundamental.

They don't all have to be committed at once.  The 1st one can stand on its own.
 2-6 are the 1st chunk, redesigning the backend.  7-9 are the 2nd chunk,
implementing {Get|Set}NamedPipeHandleState() and the FILE_PIPE_INFORMATION
stuff.  10 is the last chunk, I just don't know how to split that one without
breaking stuff.

He didn't have any problem with the tests that I recall, he just wanted the
major patch from the 1st chunk split AFAIK, so I don't think a switch is
needed.

>  it's not obvious just from the patch what's going on.  is it *only* renaming? 
> if so, what gets renamed to what?  if you only renamed one at a time, the
> number of lines being changed would be smaller, so comparisons would be easy.

The patches that say (no-op) are only renames.
part 1:
ps_connected_server => ps_connected
ps_wait_disconnect => ps_disconnected_client
client => end
part 2:
instances => numinstances
server => instance
servers => instances
event => event_empty

>  the sheer number of things being renamed here actually removes *entire*
> functions and replaces them with ones that are... identical?  are they?  i
> don't know, and i can't tell.

If you use a diff tool that shows changes inside lines it's easier.  I'd rather
not create a whole bunch of no-op patches unless Alexandre thinks it's
necessary.

>  this would at least allow those to be done as entirely independent separate
> patches that would clearly have nothing to do with the actual functionality.

That's what the (no-op) in the description is for ;-)

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.