Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Windows] Fix for net_io_counters wrapping after 4.3GB due to MIB_IFROW #835

Merged
merged 14 commits into from
Jun 12, 2016
Merged

[Windows] Fix for net_io_counters wrapping after 4.3GB due to MIB_IFROW #835

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2417702
Fix for windows net_io_counters wrapping after 4.3GB due to MIB_IFROW…
jomann09 Jun 8, 2016
bd251f2
Update HISTORY.rst with #816 issue bug fix
jomann09 Jun 9, 2016
e8cce4a
Fixes for compiling on Win XP/Win 200
jomann09 Jun 10, 2016
474b13c
Add back in ws2tcpip.h in the proper place in Win XP / Win 2000
jomann09 Jun 10, 2016
97a853c
Whoops, whitespace issue
jomann09 Jun 10, 2016
2594880
Tried to keep the mingw32 support but win 7 sdk is causing issues
jomann09 Jun 10, 2016
efb026a
Actually does need it in XP/2000 unfortunately
jomann09 Jun 10, 2016
cd3003f
Add comment lines to ifs for win versions
jomann09 Jun 10, 2016
2815aa3
Fix disk_usage test to use 1 kB block size due to issues with OS X
jomann09 Jun 10, 2016
fb6a0fc
Continue on RuntimeError when running df on partitions it can't run on
jomann09 Jun 11, 2016
76056c2
Type fix
jomann09 Jun 11, 2016
7a8c268
PEP 8 compliance and history update (Vista+ only for fix)
jomann09 Jun 11, 2016
c105688
Styling fixes (spaces instead of tabs)
jomann09 Jun 11, 2016
7769dbc
Updated to use better if/else/endif values (my bad)
jomann09 Jun 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Bug tracker at https://github.com/giampaolo/psutil/issues
- #823: [NetBSD] virtual_memory() raises TypeError on Python 3.
- #829: [UNIX] psutil.disk_usage() percent field takes root reserved space
into account.
- #816: [Windows] fixed net_io_counter() wrapping after 4.3GB in any fields
on Windows Vista and above.


4.2.0 - 2016-05-14
Expand Down
44 changes: 41 additions & 3 deletions psutil/_psutil_windows.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
#include <tchar.h>
#include <tlhelp32.h>
#include <winsock2.h>
#if (_WIN32_WINNT >= 0x0600) // Windows Vista, 7, 8, 8.1, 10
#include <ws2tcpip.h>
#endif
#include <iphlpapi.h>
#include <wtsapi32.h>
#if (_WIN32_WINNT < 0x0600) // Windows XP / 2000
#include <ws2tcpip.h>
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand: you include the same module no matter what the windows version is so why using ifdef in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually doesn't need to be included. The issue was that it matters where it's located in the list of includes, but after further testing, this header is not required for Win XP.

#include <Winsvc.h>

// Link with Iphlpapi.lib
Expand Down Expand Up @@ -85,6 +90,7 @@ typedef struct _DISK_PERFORMANCE_WIN_2008 {

// --- network connections mingw32 support
#ifndef _IPRTRMIB_H
#if (_WIN32_WINNT < 0x0600) // Windows XP / 2000
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just specify "Windows XP". Win 2000 support is long gone.

typedef struct _MIB_TCP6ROW_OWNER_PID {
UCHAR ucLocalAddr[16];
DWORD dwLocalScopeId;
Expand All @@ -101,6 +107,7 @@ typedef struct _MIB_TCP6TABLE_OWNER_PID {
MIB_TCP6ROW_OWNER_PID table[ANY_SIZE];
} MIB_TCP6TABLE_OWNER_PID, *PMIB_TCP6TABLE_OWNER_PID;
#endif
#endif

#ifndef __IPHLPAPI_H__
typedef struct in6_addr {
Expand Down Expand Up @@ -128,6 +135,7 @@ typedef struct _MIB_UDPTABLE_OWNER_PID {
} MIB_UDPTABLE_OWNER_PID, *PMIB_UDPTABLE_OWNER_PID;
#endif

#if (_WIN32_WINNT < 0x0600) // Windows XP / 2000
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just specify "Windows XP". Win 2000 support is long gone.

typedef struct _MIB_UDP6ROW_OWNER_PID {
UCHAR ucLocalAddr[16];
DWORD dwLocalScopeId;
Expand All @@ -139,6 +147,7 @@ typedef struct _MIB_UDP6TABLE_OWNER_PID {
DWORD dwNumEntries;
MIB_UDP6ROW_OWNER_PID table[ANY_SIZE];
} MIB_UDP6TABLE_OWNER_PID, *PMIB_UDP6TABLE_OWNER_PID;
#endif

PIP_ADAPTER_ADDRESSES
psutil_get_nic_addresses() {
Expand Down Expand Up @@ -1636,7 +1645,6 @@ psutil_net_connections(PyObject *self, PyObject *args) {
}

// TCP IPv6

if ((PySequence_Contains(py_af_filter, _AF_INET6) == 1) &&
(PySequence_Contains(py_type_filter, _SOCK_STREAM) == 1))
{
Expand Down Expand Up @@ -2168,7 +2176,12 @@ psutil_disk_usage(PyObject *self, PyObject *args) {
static PyObject *
psutil_net_io_counters(PyObject *self, PyObject *args) {
DWORD dwRetVal = 0;
#if (_WIN32_WINNT >= 0x0600) // Windows Vista, 7, 8, 8.1, 10
MIB_IF_ROW2 *pIfRow = NULL;
#endif
#if (_WIN32_WINNT < 0x0600) // Windows 2000 / XP
MIB_IFROW *pIfRow = NULL;
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use a single ifdef/else/endif block here.

PIP_ADAPTER_ADDRESSES pAddresses = NULL;
PIP_ADAPTER_ADDRESSES pCurrAddresses = NULL;
PyObject *py_retdict = PyDict_New();
Expand All @@ -2185,20 +2198,43 @@ psutil_net_io_counters(PyObject *self, PyObject *args) {
while (pCurrAddresses) {
py_nic_name = NULL;
py_nic_info = NULL;
#if (_WIN32_WINNT >= 0x0600) // Windows Vista, 7, 8, 8.1, 10
pIfRow = (MIB_IF_ROW2 *) malloc(sizeof(MIB_IF_ROW2));
#endif
#if (_WIN32_WINNT < 0x0600) // Windows 2000 / XP
pIfRow = (MIB_IFROW *) malloc(sizeof(MIB_IFROW));
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here


if (pIfRow == NULL) {
PyErr_NoMemory();
goto error;
}

#if (_WIN32_WINNT >= 0x0600) // Windows Vista, 7, 8, 8.1, 10
SecureZeroMemory((PVOID)pIfRow, sizeof(MIB_IF_ROW2));
pIfRow->InterfaceIndex = pCurrAddresses->IfIndex;
dwRetVal = GetIfEntry2(pIfRow);
#endif
#if (_WIN32_WINNT < 0x0600) // Windows 2000 / XP
pIfRow->dwIndex = pCurrAddresses->IfIndex;
dwRetVal = GetIfEntry(pIfRow);
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and here

if (dwRetVal != NO_ERROR) {
PyErr_SetString(PyExc_RuntimeError, "GetIfEntry() failed.");
PyErr_SetString(PyExc_RuntimeError, "GetIfEntry() or GetIfEntry2() failed.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line > 80 chars; please indent this

goto error;
}

#if (_WIN32_WINNT >= 0x0600) // Windows Vista, 7, 8, 8.1, 10
py_nic_info = Py_BuildValue("(KKKKKKKK)",
pIfRow->OutOctets,
pIfRow->InOctets,
pIfRow->OutUcastPkts,
pIfRow->InUcastPkts,
pIfRow->InErrors,
pIfRow->OutErrors,
pIfRow->InDiscards,
pIfRow->OutDiscards);
#endif
#if (_WIN32_WINNT < 0x0600) // Windows 2000 / XP
py_nic_info = Py_BuildValue("(kkkkkkkk)",
pIfRow->dwOutOctets,
pIfRow->dwInOctets,
Expand All @@ -2208,6 +2244,8 @@ psutil_net_io_counters(PyObject *self, PyObject *args) {
pIfRow->dwOutErrors,
pIfRow->dwInDiscards,
pIfRow->dwOutDiscards);
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a single ifdef/else/endif block


if (!py_nic_info)
goto error;

Expand Down
16 changes: 11 additions & 5 deletions psutil/tests/test_posix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,19 +314,25 @@ def test_os_waitpid_bad_ret_status(self):

def test_disk_usage(self):
def df(device):
out = sh("df -B 1 %s" % device).strip()
# Use 1 kB block sizes since OS X doesn't have -B flag
out = sh("df -k %s" % device).strip()
line = out.split('\n')[1]
fields = line.split()
total = int(fields[1])
used = int(fields[2])
free = int(fields[3])
total = int(fields[1]) * 1024
used = int(fields[2]) * 1024
free = int(fields[3]) * 1024
percent = float(fields[4].replace('%', ''))
return (total, used, free, percent)

tolerance = 4 * 1024 * 1024 # 4MB
for part in psutil.disk_partitions(all=False):
usage = psutil.disk_usage(part.mountpoint)
total, used, free, percent = df(part.device)
try:
total, used, free, percent = df(part.device)
except RuntimeError:
# Issue with OS X not being able to read certain partitions
# Issue with Linux systems not able to read Docker mappings
continue
Copy link
Owner

@giampaolo giampaolo Jun 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this as it can potentially continue all iterations practially skipping the whole test. You should use some other logic rather than the mere RuntimeError catch. Specifically you should inspect the cmd stderr which you can pass along the RuntimeError exception. Since this is probably too much troubles I would revert this change as it's unrelated with the Windows issue (feel free to open a new PR if you want).

self.assertAlmostEqual(usage.total, total, delta=tolerance)
self.assertAlmostEqual(usage.used, used, delta=tolerance)
self.assertAlmostEqual(usage.free, free, delta=tolerance)
Expand Down