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