[v5,2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
diff mbox

Message ID 20180608160355.67712eb8@vmware.local.home
State New
Headers show

Commit Message

Steven Rostedt June 8, 2018, 8:03 p.m. UTC
On Thu, 7 Jun 2018 11:18:16 +0200
Johan Hovold <johan@kernel.org> wrote:


> If you want to work around the warning and think you can do it in some
> non-contrived way, then go for it.
> 
> Clearing the request buffer, checking for termination using strnlen, and
> then using memcpy might not be too bad.
> 
> But after all, it is a false positive, so leaving things as they stand
> is fine too.

Not sure how contrived you think this is, but it solves the warning
without adding extra work in the normal case.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Johan Hovold June 11, 2018, 3:46 p.m. UTC | #1
On Fri, Jun 08, 2018 at 04:03:55PM -0400, Steven Rostedt wrote:
> On Thu, 7 Jun 2018 11:18:16 +0200
> Johan Hovold <johan@kernel.org> wrote:
> 
> 
> > If you want to work around the warning and think you can do it in some
> > non-contrived way, then go for it.
> > 
> > Clearing the request buffer, checking for termination using strnlen, and
> > then using memcpy might not be too bad.
> > 
> > But after all, it is a false positive, so leaving things as they stand
> > is fine too.
> 
> Not sure how contrived you think this is, but it solves the warning
> without adding extra work in the normal case.
> 
> -- Steve
> 
> diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
> index 71aec14f8181..4fb9f1dff47d 100644
> --- a/drivers/staging/greybus/fw-management.c
> +++ b/drivers/staging/greybus/fw-management.c
> @@ -150,15 +150,18 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
>  	}
>  
>  	request.load_method = load_method;
> -	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> +	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
>  
>  	/*
>  	 * The firmware-tag should be NULL terminated, otherwise throw error and
>  	 * fail.
>  	 */
> -	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> -		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> -		return -EINVAL;
> +	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
> +		if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> +			dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> +			return -EINVAL;
> +		}
> +		request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
>  	}

Well, I think it's quite far from obvious what is going on above, and
not least why things are being done this way (which a comment may help
with).

And just NUL-terminating the (automatic) buffer before returning wasn't
enough? Then it may be better to do away with strncpy completely.

But should we really be working around gcc this way? If the
implementation of this new warning isn't smart enough yet, should it not
just be disabled instead?

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 71aec14f8181..4fb9f1dff47d 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -150,15 +150,18 @@  static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
 	}
 
 	request.load_method = load_method;
-	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
+	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
 
 	/*
 	 * The firmware-tag should be NULL terminated, otherwise throw error and
 	 * fail.
 	 */
-	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
-		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
-		return -EINVAL;
+	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
+		if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
+			dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
+			return -EINVAL;
+		}
+		request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
 	}
 
 	/* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */