linux-kernel - checkpatch potential false positive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171106041914.GB18478@eros>
Date:   Mon, 6 Nov 2017 15:19:14 +1100
From:   "Tobin C. Harding" <me@...in.cc>
To:     Andy Whitcroft <apw@...onical.com>, Joe Perches <joe@...ches.com>
Cc:     linux-kernel@...r.kernel.org
Subject: checkpatch potential false positive

Hi,

When parsing drivers/staging/unisys/visorbus/visorchipset.c in Greg's
staging tree checkpatch emits

--------------
visorchipset.c
--------------
WARNING: char * array declaration might be better as static const
#1050: FILE: visorchipset.c:1050:
+	char *envp[] = { env_cmd, env_id, env_state, env_bus, env_dev,

WARNING: char * array declaration might be better as static const
#1140: FILE: visorchipset.c:1140:
+	char *envp[] = { env_selftest, NULL };

total: 0 errors, 2 warnings, 1694 lines checked

I may be wrong but I think the code in question is clean and
correct. Since checkpatch is saying this _might_ be better ... perhaps
checkpatch could emit CHECK instead of WARNING for this?

Here is the complete function to save finding the file.

/*
 * parahotplug_request_kickoff() - initiate parahotplug request
 * @req: the request to initiate
 *
 * Cause uevent to run the user level script to do the disable/enable specified
 * in the parahotplug_request.
 */
static int parahotplug_request_kickoff(struct parahotplug_request *req)
{
	struct controlvm_message_packet *cmd = &req->msg.cmd;
	char env_cmd[40], env_id[40], env_state[40], env_bus[40], env_dev[40],
	     env_func[40];
	char *envp[] = { env_cmd, env_id, env_state, env_bus, env_dev,
			 env_func, NULL
	};

	sprintf(env_cmd, "VISOR_PARAHOTPLUG=1");
	sprintf(env_id, "VISOR_PARAHOTPLUG_ID=%d", req->id);
	sprintf(env_state, "VISOR_PARAHOTPLUG_STATE=%d",
		cmd->device_change_state.state.active);
	sprintf(env_bus, "VISOR_PARAHOTPLUG_BUS=%d",
		cmd->device_change_state.bus_no);
	sprintf(env_dev, "VISOR_PARAHOTPLUG_DEVICE=%d",
		cmd->device_change_state.dev_no >> 3);
	sprintf(env_func, "VISOR_PARAHOTPLUG_FUNCTION=%d",
		cmd->device_change_state.dev_no & 0x7);
	return kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj,
				  KOBJ_CHANGE, envp);
}

If this is not the sort of bug report you like, could you please (if you
have time) school me on how to better report such things in the future.

thanks,
Tobin.