On Wednesday 19 August 2015 00:57:20 Claudiu Olteanu wrote: > Hi there, > > As I promised, I created a new set of patches. You can find it > attached to this e-mail. > > Here is a list with the changes I did after the feedback: > - initialized the internal variables (patch 06)
Thanks! > - moved the *WSACleanup* call from patch 06 to patch 12 Good, like I said that is now better than Qt itself. > - removed the implementation of my internal *getLastError* and started > using *qt_error_string* (patch 11) > - used *WSAAddressToStringW* instead of *WSAAddressToStringA* (patch 11) > - avoided some code duplication (patches 14, 15, 17) > > I hope that I covered all of Thiago's suggestions. Yes, it does. > Also I didn't remove the call of strdup method from > *qtserialbluetooth::qt_serial_open *because the > *WSAStringToAddressA* method is expecting to receive > LPSTR {aka char*} parameter while I have the address > represented as const char*. The Win32 API documentation says it's an input parameter. It's probably a mistake on MSFT's part to have forgotten the "const" there. But let's not tempt fate and we'll leave it with the strdup. > I tried as well to replace the WSAStringToAddressA with > WSAStringToAddressW but I had to represent the > address as a wchar_t* and and when I wanted to use > mbstowcs_s for conversion (from const char* to wchar_t*) > the compiler couldn't find the declaration to the method > (even though I included the stdlib header). After some > failed attempts I gave up :). Like I said, never, ever use <wchar.h> functions. Those are braindead in design, at least compared to Qt's equivalents. Before I go into this, let me say that the patches are fine and Dirk can apply them as-is. There's no need to change further. With that now said, here's how you can improve. Your code is: > + char *address = strdup(devaddr); [...] > + if (WSAStringToAddressA(address, > + AF_BTH, > + NULL, > + (LPSOCKADDR) &socketBthAddress, > + &socketBthAddressBth > + ) != 0) { > + To use the W function: QString address = QString::fromLatin1(devaddr); if (WSAStringToAddressW(reinterpret_cast<wchar_t*>(address.utf16()), [...] This also solves the strdup() and free() calls. And if the line didn't get too long, you could do everything in one line. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface