Review request: ~200 build warning fixes to kdebase
Tobias Koenig
tokoe at kde.org
Sat Jun 5 12:34:28 BST 2010
On Sat, Jun 05, 2010 at 02:34:36AM -0700, Xander Moore wrote:
Hej Xander,
> --- runtime/kioslave/network/network/builder/upnp/upnpnetworkbuilder.cpp (revision 1132570)
> +++ runtime/kioslave/network/network/builder/upnp/upnpnetworkbuilder.cpp (working copy)
> @@ -110,7 +110,7 @@
> const QString ipAddress = upnpDevice.ipAddress();
>
> NetDevicePrivate* d = 0;
> - const NetDevice* deviceOfService;
> + const NetDevice* deviceOfService = NULL;
> foreach( const NetDevice& device, deviceList )
> {
> const bool isSameAddress = ( device.ipAddress() == ipAddress );
Please use
const NetDevice* deviceOfService = 0;
NULL is C-ish and shouldn't be used in C++ code according to our coding style.
> --- runtime/kglobalaccel/globalshortcutsregistry.cpp (revision 1132570)
> +++ runtime/kglobalaccel/globalshortcutsregistry.cpp (working copy)
> @@ -215,14 +215,14 @@
> data.append(shortcut->friendlyName());
> #ifdef Q_WS_X11
> // pass X11 timestamp
> - long timestamp = QX11Info::appTime();
> +// long timestamp = QX11Info::appTime();
> // Make sure kglobalacceld has ungrabbed the keyboard after receiving the
> // keypress, otherwise actions in application that try to grab the
> // keyboard (e.g. in kwin) may fail to do so. There is still a small race
> // condition with this being out-of-process.
> qApp->syncX();
> #else
> - long timestamp = 0;
> +// long timestamp = 0;
> #endif
'timestamp' is not used in the method? If not, just remove it, commented code
is just confusing.
> Index: workspace/kcheckpass/kcheckpass.c
> ===================================================================
> --- workspace/kcheckpass/kcheckpass.c (revision 1132570)
> +++ workspace/kcheckpass/kcheckpass.c (working copy)
> @@ -420,7 +420,12 @@
> }
>
> lseek(lfd, 0, SEEK_SET);
> - write(lfd, fcont, sprintf(fcont, "%lu\n", time(0) + THROTTLE));
> + int count = sprintf(fcont, "%lu\n", time(0) + THROTTLE);
> + if (write(lfd, fcont, count) != count)
> + {
> + message("Failed to write entire buffer.");
> + return AuthError;
> + }
>
> close(lfd);
> }
This could be written as
const int count = sprintf(fcont, "%lu\n", time(0) + THROTTLE);
or? Just to make clear that this variable won't be modified anymore...
(there might be some other places in your patch where it should be fixed)
Thanks for the patch, I can imagine it was a lot of work to create it! :)
Ciao,
Tobias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100605/3ededb9d/attachment.sig>
More information about the kde-core-devel
mailing list