[1/5] discover: Fix bad check of version string
diff mbox series

Message ID 20180306040220.7519-2-joel@jms.id.au
State Accepted
Headers show
Series
  • Misc fixes
Related show

Commit Message

Joel Stanley March 6, 2018, 4:02 a.m. UTC
Clang says this:

discover/device-handler.c:1564:27: warning: size argument in 'strncmp' call is a comparison [-Wmemsize-comparison]
                                        strlen(opt->version) == 0)) {
                                        ~~~~~~~~~~~~~~~~~~~~~^~~~
discover/device-handler.c:1563:5: note: did you mean to compare the result of 'strncmp' instead?
                                strncmp(opt->version, tmp->version,
                                ^

It looks like it's correct. However, we can go one better and drop the
pointless strncmp(foo, bar, strlen(bar)), as this is equivalent to
strcmp(foo, bar).

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 discover/device-handler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Cyril Bur March 6, 2018, 4:44 a.m. UTC | #1
On Tue, 2018-03-06 at 14:32 +1030, Joel Stanley wrote:
> Clang says this:
> 
> discover/device-handler.c:1564:27: warning: size argument in 'strncmp' call is a comparison [-Wmemsize-comparison]
>                                         strlen(opt->version) == 0)) {
>                                         ~~~~~~~~~~~~~~~~~~~~~^~~~
> discover/device-handler.c:1563:5: note: did you mean to compare the result of 'strncmp' instead?
>                                 strncmp(opt->version, tmp->version,
>                                 ^
> 
> It looks like it's correct. However, we can go one better and drop the
> pointless strncmp(foo, bar, strlen(bar)), as this is equivalent to
> strcmp(foo, bar).

I'm in favour of this equivalence!

> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

> ---
>  discover/device-handler.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index a6eb8843d514..569e65290bd7 100644
> --- a/discover/device-handler.c
> +++ b/discover/device-handler.c
> @@ -1560,8 +1560,7 @@ void device_handler_add_plugin_option(struct device_handler *handler,
>  		tmp = handler->plugins[i];
>  		/* If both id and version match, ignore */
>  		if (strncmp(opt->id, tmp->id, strlen(opt->id)) == 0 &&
> -				strncmp(opt->version, tmp->version,
> -					strlen(opt->version) == 0)) {
> +				strcmp(opt->version, tmp->version) == 0) {
>  			pb_log("discover: Plugin '%s' already exists, ignoring\n",
>  					opt->id);
>  			return;

Patch
diff mbox series

diff --git a/discover/device-handler.c b/discover/device-handler.c
index a6eb8843d514..569e65290bd7 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -1560,8 +1560,7 @@  void device_handler_add_plugin_option(struct device_handler *handler,
 		tmp = handler->plugins[i];
 		/* If both id and version match, ignore */
 		if (strncmp(opt->id, tmp->id, strlen(opt->id)) == 0 &&
-				strncmp(opt->version, tmp->version,
-					strlen(opt->version) == 0)) {
+				strcmp(opt->version, tmp->version) == 0) {
 			pb_log("discover: Plugin '%s' already exists, ignoring\n",
 					opt->id);
 			return;