August 2015 - Wine-Devel - list.winehq.org

Re: [PATCH 5/5] d3d11/tests: Add test for 2D texture interfaces.

by Józef Kucia

On Fri, Aug 28, 2015 at 5:27 AM, Marvin <testbot(a)winehq.org> wrote: > Hi, > > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > https://testbot.winehq.org/JobDetails.pl?Key=16380 > > Your paranoid android. > > > === w7u (32 bit d3d11) === > d3d11.c:360: Test failed: 0: Texture should implement ID3D10Texture2D. > d3d11.c:352: Test failed: 2: Failed to create a 2d texture, hr 0x8007000e. > > === w8 (32 bit d3d11) === > d3d11.c:360: Test failed: 0: Texture should implement ID3D10Texture2D. > > D3D11 textures created with the D3D11_RESOURCE_MISC_RESOURCE_CLAMP flag seem to not implement ID3D10Texture2D in WARP devices. I'll resend the patch later.

Aug. 28, 2015

Re: ieframe: Fixed random test failures.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=16383 Your paranoid android. === w2000pro (32 bit webbrowser) === webbrowser.c:2972: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEBACK_FALSE webbrowser.c:2977: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEFORWARD_FALSE webbrowser.c:2972: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEBACK_FALSE webbrowser.c:2977: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEFORWARD_FALSE webbrowser.c:2972: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEBACK_FALSE webbrowser.c:2977: Test failed: expected Invoke_COMMANDSTATECHANGE_NAVIGATEFORWARD_FALSE

Aug. 28, 2015

Re: What would the best approach for adding securing Wine and is it a reasonable goal?

by Michael Stefaniuc

Hello Steven, On 08/28/2015 02:44 AM, Steven Stewart-Gallus wrote: > I noticed that CreateRestrictedToken basically just duplicates the token and > doesn't do anything. This makes sense as Wine is implemented as a compatibility > library and program loader and not as a virtual machine and Windows programs > running on Wine can break out and access Linux system calls so making > CreateRestrictedToken do anything more wouldn't accomplish anything. I think it > would be really cool though if CreateRestrictedToken and similar sandboxing APIs > actually accomplished something in Wine though. > > What would the best approach for securing Wine be and is it a reasonable goal? the easy point that people miss: In this context Wine is an OS and what the people want is to secure their application. Most attacks are targeting applications and not the OS. So like on Linux it depends on the application on how to secure it. So what helps to secure an app will break an other. Is it a reasonable goal? Sure, if it scratches your itch and you want to work on it then go ahead, nobody will stop you. A big part of this can be developed outside of upstream Wine. You may want to check out ZeroWine as that is a specialized project to analyze Windows malware in a Wine sandbox. But for the Wine project in general focusing on securing Wine has a huge opportunity cost. Most of the Wine project members are working on getting applications to run and run well under Wine. There is more than enough work left there. bye michael > First, see http://wiki.winehq.org/SecuringWine. So far, I can think of a few > approaches. The first approach is to make a whole operating system such as > ReactOS. The second approach is making a whole operating system that only has > device drivers for working on top of virtual machines or hypervisors. The third > approach is making a kernel that runs as a normal user process on top of a full > OS (similar to User-Mode Linux or a weekend project I am working on at > https://gitlab.com/sstewartgallus/wum, also see the Chrome sandbox at > https://www.chromium.org/developers/design-documents/sandbox, > https://code.google.com/p/chromium/wiki/LinuxSandboxing and > https://dev.chromium.org/developers/design-documents/sandbox/osx-sandboxing…) > The fourth approach is to abandon securing Wine itself and simply run Wine > programs inside separate sandboxes. > > ReactOS already serves as an example of the first approach. > > The second approach is basically similar to ReactOS. > > My project at https://gitlab.com/sstewartgallus/wum serves as a tiny example of > of the third approach might work. > > The fourth approach of separate sandboxes is actually very simple. One possible > implementation of this fourth approach on Linux might be to use a script like > the sandbox-X.sh script shown below and remove the Z: symlink in dosdevices > (this isn't actually needed or improves security but just looks nice.) Note that > to make this script work I had to do some configuration to let my automatically > su into the sandbox account. As well, I also had to download a dummy Xorg > configuration to the home directory of the sandbox account. > > Thank you, > Steven Stewart-Gallus > > sandbox-X.sh: > #! /bin/sh > > set -e > set -C > set -u > > exec su -l -c 'exec sh <&3' sstewartgallus-sandbox 3<<'EOF' > set -e > set -C > set -u > > exec 3<&- > > export DISPLAY=':76' > export XPRA_SOCKET_DIR='/tmp' > exec xpra\ > start\ > ${DISPLAY}\ > --xvfb="Xorg -noreset +extension GLX +extension RANDR +extension RENDER > -logfile ./${DISPLAY}.log -config ./xorg.conf"\ > "--no-daemon"\ > "--mmap-group"\ > "--no-sharing"\ > "--bell"\ > "--system-tray"\ > "--xsettings"\ > "--cursors"\ > "--encoding=rgb"\ > "--clipboard"\ > --start-child=xterm > > >

Aug. 28, 2015

Re: [PATCH 1/2] vcruntime140: Add the new MSVC 2015 compiler specific DLL.

by Alexandre Julliard

Martin Storsjo <martin(a)martin.st> writes: > Since MSVC 2015/Windows 10, the C runtime has now been split into two > parts, ucrtbase, which is the generic C runtime which is now considered > a system component, and vcruntimeX which is specific to the compiler > version. > > Even if this DLL only has a fraction of the functions of the full > msvcrt/ucrtbase, it still requires almost all the object files from > msvcrt. It would be better to isolate the corresponding code so that we don't need all the files. -- Alexandre Julliard julliard(a)winehq.org

Aug. 28, 2015

Re: ws2_32: WSACleanup cleans up open sockets (OSX only)

by Matt Durgavich

Another try, this time server side. It was much simpler to do --- dlls/ws2_32/socket.c | 13 ++++++++++++- server/handle.c | 12 ++++++++++++ server/protocol.def | 3 +++ server/sock.c | 2 +- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index ca82ec9..66d5b2b 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1471,7 +1471,18 @@ INT WINAPI WSACleanup(void) { if (num_startup) { num_startup--; - TRACE("pending cleanups: %d\n", num_startup); + if (num_startup == 0) + { + SERVER_START_REQ(close_all_sockets) + { + wine_server_call( req ); + } + SERVER_END_REQ; + } + else + { + TRACE("pending cleanups: %d\n", num_startup); + } return 0; } SetLastError(WSANOTINITIALISED); diff --git a/server/handle.c b/server/handle.c index 5043ff7..b4b5ecb 100644 --- a/server/handle.c +++ b/server/handle.c @@ -39,6 +39,8 @@ #include "security.h" #include "request.h" +extern struct object_ops sock_ops; + struct handle_entry { struct object *ptr; /* object */ @@ -608,6 +610,16 @@ DECL_HANDLER(set_handle_info) reply->old_flags = set_handle_flags( current->process, req->handle, req->mask, req->flags ); } +DECL_HANDLER(close_all_sockets) +{ + obj_handle_t sock; + UINT index = 0; + while ( (sock = enumerate_handles(current->process, &sock_ops, &index)) ) + { + close_handle(current->process, sock); + } +} + /* duplicate a handle */ DECL_HANDLER(dup_handle) { diff --git a/server/protocol.def b/server/protocol.def index c313006..651af47 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -942,6 +942,9 @@ struct rawinput_device obj_handle_t handle; /* handle to close */ @END +/* Close all sockets for the current process */ +(a)REQ(close_all_sockets) +(a)END /* Set a handle information */ @REQ(set_handle_info) diff --git a/server/sock.c b/server/sock.c index 67d6416..2a9f444 100644 --- a/server/sock.c +++ b/server/sock.c @@ -138,7 +138,7 @@ static int sock_get_ntstatus( int err ); static int sock_get_error( int err ); static void sock_set_error(void); -static const struct object_ops sock_ops = +const struct object_ops sock_ops = { sizeof(struct sock), /* size */ sock_dump, /* dump */ -- 2.3.2 (Apple Git-55) > On Aug 27, 2015, at 9:58 AM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > > That indeed sounds much easier than having to keep our own list inside the dll. Thanks, Henri. > > Bruno > > On Thursday, August 27, 2015, Matt Durgavich <mattdurgavich(a)gmail.com <mailto:mattdurgavich(a)gmail.com>> wrote: > Cool. I'll whip up another patch with this approach. I thought this piece of work would be straightforward ;) > > Thanks! > > Best, > -Matt > > > On Thu, Aug 27, 2015 at 8:47 AM, Henri Verbeet <hverbeet(a)gmail.com <javascript:_e(%7B%7D,'cvml','hverbeet(a)gmail.com');>> wrote: > > On 27 August 2015 at 14:09, Bruno Jesus <00cpxxx(a)gmail.com <javascript:_e(%7B%7D,'cvml','00cpxxx(a)gmail.com');>> wrote: > > I have other email asking how to list all sockets from inside the server but > > I didn't get any replies so at that time I thought it was not possible to do > > that. > You'd iterate over current->process->handles->entries and compare > entry->ptr->ops against sock_ops. I.e., enumerate_handles() in > server/handle.c, although that seems currently unused. > >

Aug. 28, 2015

Re: [PATCH 5/5] d3d11/tests: Add test for 2D texture interfaces.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=16380 Your paranoid android. === w7u (32 bit d3d11) === d3d11.c:360: Test failed: 0: Texture should implement ID3D10Texture2D. d3d11.c:352: Test failed: 2: Failed to create a 2d texture, hr 0x8007000e. === w8 (32 bit d3d11) === d3d11.c:360: Test failed: 0: Texture should implement ID3D10Texture2D.

Aug. 28, 2015

What would the best approach for adding securing Wine and is it a reasonable goal?

by Steven Stewart-Gallus

Hello, I noticed that CreateRestrictedToken basically just duplicates the token and doesn't do anything. This makes sense as Wine is implemented as a compatibility library and program loader and not as a virtual machine and Windows programs running on Wine can break out and access Linux system calls so making CreateRestrictedToken do anything more wouldn't accomplish anything. I think it would be really cool though if CreateRestrictedToken and similar sandboxing APIs actually accomplished something in Wine though. What would the best approach for securing Wine be and is it a reasonable goal? First, see http://wiki.winehq.org/SecuringWine. So far, I can think of a few approaches. The first approach is to make a whole operating system such as ReactOS. The second approach is making a whole operating system that only has device drivers for working on top of virtual machines or hypervisors. The third approach is making a kernel that runs as a normal user process on top of a full OS (similar to User-Mode Linux or a weekend project I am working on at https://gitlab.com/sstewartgallus/wum, also see the Chrome sandbox at https://www.chromium.org/developers/design-documents/sandbox, https://code.google.com/p/chromium/wiki/LinuxSandboxing and https://dev.chromium.org/developers/design-documents/sandbox/osx-sandboxing…) The fourth approach is to abandon securing Wine itself and simply run Wine programs inside separate sandboxes. ReactOS already serves as an example of the first approach. The second approach is basically similar to ReactOS. My project at https://gitlab.com/sstewartgallus/wum serves as a tiny example of of the third approach might work. The fourth approach of separate sandboxes is actually very simple. One possible implementation of this fourth approach on Linux might be to use a script like the sandbox-X.sh script shown below and remove the Z: symlink in dosdevices (this isn't actually needed or improves security but just looks nice.) Note that to make this script work I had to do some configuration to let my automatically su into the sandbox account. As well, I also had to download a dummy Xorg configuration to the home directory of the sandbox account. Thank you, Steven Stewart-Gallus sandbox-X.sh: #! /bin/sh set -e set -C set -u exec su -l -c 'exec sh <&3' sstewartgallus-sandbox 3<<'EOF' set -e set -C set -u exec 3<&- export DISPLAY=':76' export XPRA_SOCKET_DIR='/tmp' exec xpra\ start\ ${DISPLAY}\ --xvfb="Xorg -noreset +extension GLX +extension RANDR +extension RENDER -logfile ./${DISPLAY}.log -config ./xorg.conf"\ "--no-daemon"\ "--mmap-group"\ "--no-sharing"\ "--bell"\ "--system-tray"\ "--xsettings"\ "--cursors"\ "--encoding=rgb"\ "--clipboard"\ --start-child=xterm

Aug. 28, 2015

Re: [PATCH 2/4] ntdll: Fix read_directory_getdirentries().

by Ken Thomases

On Aug 27, 2015, at 5:05 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > > 2015-08-27 23:27 GMT+02:00 Ken Thomases <ken(a)codeweavers.com>: >> On Aug 27, 2015, at 2:08 PM, Matteo Bruni <mbruni(a)codeweavers.com> wrote: >>> >>> --- >>> The man page for getdirentries mentions that only positions previously >>> returned from lseek or getdirentries should be used, I'm arguably abusing >>> the pointer to store the index and then restoring the position using >>> SEEK_CUR. It seems to work fine for me, not sure if that might cause >>> issues with some specific filesystem setups. >> >> You're not restoring it, you're setting it to a computed value. The computation assumes that the file position is the index of the entry that was read. For example, you get the current position and then you're setting the position to that original position minus 2 (for the synthesized . and .. entries). That doesn't seem like a reasonable assumption to me. >> >> In some quick testing (with different code), I get results like these: >> >> rc 8172 base 0/0x00000000 filepos 469762300/0x1c0000fc entries 252/0x000000fc total 252/0x000000fc >> rc 8188 base 469762300/0x1c0000fc filepos 469762640/0x1c000250 entries 340/0x00000154 total 592/0x00000250 >> rc 2364 base 469762640/0x1c000250 filepos 469762732/0x1c0002ac entries 92/0x0000005c total 684/0x000002ac >> >> The file positions seem to be 0x1c000000 with the entry index added to it. However, each run of my test program gives a different fixed baseline. The file positions are definitely not _just_ entry indices. >> >> It does seem to work to seek to an entry index in some quick testing, but it makes me very nervous. > > Yes, I didn't explain it clearly but that's what I meant with the > commit message. More below. I did some poking through the OS X kernel code. Different file systems seem to interpret the file position differently when reading directory entries. So, I'm more convinced than before that assuming you can compute a file position from the desired directory entry index won't work in the general case. The observed scheme seems to be specific to the HFS(+) driver. > A "safer" option would be to store the position in the fd unchanged > but reserve two values (-1 and -2?) for the '.' and '..'. I'm not sure > that's entirely safe but I don't see anything better. Negative values are rejected by lseek() with EINVAL. (Didn't actually try it, but the man page says so and the kernel code seemed to agree. ;) We would need to treat offset 0 as indicating that the next entry to be read is ".". OFF_MAX - 1 would indicate that the next entry to be read is "..". OFF_MAX - 2 would indicate that we should seek to 0 and read real entries. Or something like that. I think we should not use OFF_MAX itself as a magic value. In my testing, the NFS driver sets the position to that at EOF. Of course, that raises the possibility that there are other special values that we'll collide with. -Ken

Aug. 28, 2015

Re: [PATCH 2/4] ntdll: Fix read_directory_getdirentries().

by Matteo Bruni

2015-08-27 23:27 GMT+02:00 Ken Thomases <ken(a)codeweavers.com>: > On Aug 27, 2015, at 2:08 PM, Matteo Bruni <mbruni(a)codeweavers.com> wrote: >> >> --- >> The man page for getdirentries mentions that only positions previously >> returned from lseek or getdirentries should be used, I'm arguably abusing >> the pointer to store the index and then restoring the position using >> SEEK_CUR. It seems to work fine for me, not sure if that might cause >> issues with some specific filesystem setups. > > You're not restoring it, you're setting it to a computed value. The computation assumes that the file position is the index of the entry that was read. For example, you get the current position and then you're setting the position to that original position minus 2 (for the synthesized . and .. entries). That doesn't seem like a reasonable assumption to me. > > In some quick testing (with different code), I get results like these: > > rc 8172 base 0/0x00000000 filepos 469762300/0x1c0000fc entries 252/0x000000fc total 252/0x000000fc > rc 8188 base 469762300/0x1c0000fc filepos 469762640/0x1c000250 entries 340/0x00000154 total 592/0x00000250 > rc 2364 base 469762640/0x1c000250 filepos 469762732/0x1c0002ac entries 92/0x0000005c total 684/0x000002ac > > The file positions seem to be 0x1c000000 with the entry index added to it. However, each run of my test program gives a different fixed baseline. The file positions are definitely not _just_ entry indices. > > It does seem to work to seek to an entry index in some quick testing, but it makes me very nervous. Yes, I didn't explain it clearly but that's what I meant with the commit message. More below. >> --- >> dlls/ntdll/directory.c | 143 ++++++++++++++++++------------------------------- >> 1 file changed, 53 insertions(+), 90 deletions(-) >> >> diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c >> index f9562d2..eb6cd45 100644 >> --- a/dlls/ntdll/directory.c >> +++ b/dlls/ntdll/directory.c >> @@ -1820,27 +1820,27 @@ static int read_directory_getdirentries( int fd, IO_STATUS_BLOCK *io, void *buff >> BOOLEAN restart_scan, FILE_INFORMATION_CLASS class ) >> { >> long restart_pos; >> - ULONG_PTR restart_info_pos = 0; >> - size_t size, initial_size = length; >> - int res, fake_dot_dot = 1; >> - char *data, local_buffer[8192]; >> + off_t newcursor; > > This newcursor variable is only ever set and then never used. Right, will fix. > >> + int total, res, i; >> + char data[8192]; >> struct dirent *de; >> - union file_directory_info *info, *last_info = NULL, *restart_last_info = NULL; >> + union file_directory_info *info, *last_info = NULL; >> >> - size = initial_size; >> - data = local_buffer; >> - if (size > sizeof(local_buffer) && !(data = RtlAllocateHeap( GetProcessHeap(), 0, size ))) >> + io->u.Status = STATUS_SUCCESS; >> + >> + if (restart_scan) >> { >> - io->u.Status = STATUS_NO_MEMORY; >> - return io->u.Status; >> + lseek( fd, 0, SEEK_SET ); >> + i = 0; >> + } >> + else >> + { >> + i = lseek( fd, 0, SEEK_CUR ); >> + lseek( fd, 0, SEEK_SET ); >> + newcursor = lseek( fd, i < 2 ? 0 : i - 2, SEEK_CUR ); > > Here's where you're seeking to an arbitrary position. Also, the last two seeks can be combined (if they were right). That is, seeking to the beginning (0, SEEK_SET) and then seeking relative to the current position is just seeking to the relative position as though it were absolute. That's true only assuming that a lseek(SEEK_SET) and a lseek(SEEK_CUR) turn out to be in practice the same operation. I instead thought that they would be intrinsically different and that, while a SEEK_SET to an arbitrary position is not fine, SEEK_SET to 0 + SEEK_CUR is okay. I guess I'm wrong. A "safer" option would be to store the position in the fd unchanged but reserve two values (-1 and -2?) for the '.' and '..'. I'm not sure that's entirely safe but I don't see anything better. >> res = 0; >> done: >> - if (data != local_buffer) RtlFreeHeap( GetProcessHeap(), 0, data ); >> return res; > > This wasn't changed by your patch, but it appears that this function can only return 0 at this point, so it should just do so. res is set to 0 just before the "done" label. All "goto done" statements are immediately preceded by setting res to 0. So, the conflation of "res" with a return value should be eliminated. Setting it to 0 in those various places is unnecessary. That's right, I missed it. > Once that's cleared up, I would rename res and total to something like bytes_left and bytes_read, respectively. To me, on my first scan through your patch, I was expecting total to accumulate the total amount read over several iterations. (Not that that would have been useful, but it confused me.) Good point, I'll probably take your suggested names. Thanks for the review!

Aug. 27, 2015

Re: [PATCH 2/4] ntdll: Fix read_directory_getdirentries().

by Ken Thomases

On Aug 27, 2015, at 2:08 PM, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > > --- > The man page for getdirentries mentions that only positions previously > returned from lseek or getdirentries should be used, I'm arguably abusing > the pointer to store the index and then restoring the position using > SEEK_CUR. It seems to work fine for me, not sure if that might cause > issues with some specific filesystem setups. You're not restoring it, you're setting it to a computed value. The computation assumes that the file position is the index of the entry that was read. For example, you get the current position and then you're setting the position to that original position minus 2 (for the synthesized . and .. entries). That doesn't seem like a reasonable assumption to me. In some quick testing (with different code), I get results like these: rc 8172 base 0/0x00000000 filepos 469762300/0x1c0000fc entries 252/0x000000fc total 252/0x000000fc rc 8188 base 469762300/0x1c0000fc filepos 469762640/0x1c000250 entries 340/0x00000154 total 592/0x00000250 rc 2364 base 469762640/0x1c000250 filepos 469762732/0x1c0002ac entries 92/0x0000005c total 684/0x000002ac The file positions seem to be 0x1c000000 with the entry index added to it. However, each run of my test program gives a different fixed baseline. The file positions are definitely not _just_ entry indices. It does seem to work to seek to an entry index in some quick testing, but it makes me very nervous. > --- > dlls/ntdll/directory.c | 143 ++++++++++++++++++------------------------------- > 1 file changed, 53 insertions(+), 90 deletions(-) > > diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c > index f9562d2..eb6cd45 100644 > --- a/dlls/ntdll/directory.c > +++ b/dlls/ntdll/directory.c > @@ -1820,27 +1820,27 @@ static int read_directory_getdirentries( int fd, IO_STATUS_BLOCK *io, void *buff > BOOLEAN restart_scan, FILE_INFORMATION_CLASS class ) > { > long restart_pos; > - ULONG_PTR restart_info_pos = 0; > - size_t size, initial_size = length; > - int res, fake_dot_dot = 1; > - char *data, local_buffer[8192]; > + off_t newcursor; This newcursor variable is only ever set and then never used. > + int total, res, i; > + char data[8192]; > struct dirent *de; > - union file_directory_info *info, *last_info = NULL, *restart_last_info = NULL; > + union file_directory_info *info, *last_info = NULL; > > - size = initial_size; > - data = local_buffer; > - if (size > sizeof(local_buffer) && !(data = RtlAllocateHeap( GetProcessHeap(), 0, size ))) > + io->u.Status = STATUS_SUCCESS; > + > + if (restart_scan) > { > - io->u.Status = STATUS_NO_MEMORY; > - return io->u.Status; > + lseek( fd, 0, SEEK_SET ); > + i = 0; > + } > + else > + { > + i = lseek( fd, 0, SEEK_CUR ); > + lseek( fd, 0, SEEK_SET ); > + newcursor = lseek( fd, i < 2 ? 0 : i - 2, SEEK_CUR ); Here's where you're seeking to an arbitrary position. Also, the last two seeks can be combined (if they were right). That is, seeking to the beginning (0, SEEK_SET) and then seeking relative to the current position is just seeking to the relative position as though it were absolute. > res = 0; > done: > - if (data != local_buffer) RtlFreeHeap( GetProcessHeap(), 0, data ); > return res; This wasn't changed by your patch, but it appears that this function can only return 0 at this point, so it should just do so. res is set to 0 just before the "done" label. All "goto done" statements are immediately preceded by setting res to 0. So, the conflation of "res" with a return value should be eliminated. Setting it to 0 in those various places is unnecessary. Once that's cleared up, I would rename res and total to something like bytes_left and bytes_read, respectively. To me, on my first scan through your patch, I was expecting total to accumulate the total amount read over several iterations. (Not that that would have been useful, but it confused me.) -Ken

Aug. 27, 2015