On Thu, 2011-11-24 at 02:05 +0100, Jörn Engel wrote:
> A reader should spend an extra moment whenever noticing a cast,
> because either something special is going on that deserves extra
> attention or, as is all too often the case, the code is wrong.
>
> These casts, afaics, have all been useless. They cast a foo* to a
> foo*, cast a void* to the assigned type, cast a foo* to void*, before
> assigning it to a void* variable, etc.
>
> In a few cases I also removed an additional &...[0], which is equally
> useless.
>
> Lastly I added three FIXMEs where, to the best of my judgement, the
> code appears to have a bug. It would be good if someone could check
> these.
>
> Signed-off-by: Joern Engel <joern@xxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target.c | 16 ++++++------
> drivers/target/iscsi/iscsi_target_auth.c | 28 ++++++++++----------
> drivers/target/iscsi/iscsi_target_configfs.c | 5 +--
> drivers/target/iscsi/iscsi_target_login.c | 21 ++++++++-------
> drivers/target/iscsi/iscsi_target_nego.c | 2 +-
> drivers/target/iscsi/iscsi_target_nodeattrib.c | 2 +-
> drivers/target/iscsi/iscsi_target_stat.c | 16 ++++++------
> drivers/target/iscsi/iscsi_target_tpg.c | 2 +-
> drivers/target/iscsi/iscsi_target_util.c | 4 +-
> drivers/target/loopback/tcm_loop.c | 18 ++++---------
> drivers/target/target_core_cdb.c | 20 ++++++---------
> drivers/target/target_core_configfs.c | 18 ++++++------
> drivers/target/target_core_fabric_lib.c | 6 ++--
> drivers/target/target_core_file.c | 10 +++---
> drivers/target/target_core_iblock.c | 2 +-
> drivers/target/target_core_pscsi.c | 14 +++++-----
> drivers/target/target_core_stat.c | 3 +-
> drivers/target/target_core_stgt.c | 2 +-
> drivers/target/target_core_transport.c | 4 +-
> drivers/target/tcm_qla2xxx/tcm_qla2xxx_configfs.c | 14 ++++------
> drivers/target/tcm_vhost/tcm_vhost_configfs.c | 5 +--
> 21 files changed, 99 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c
> index beb3946..fc77c5b 100644
> --- a/drivers/target/iscsi/iscsi_target_auth.c
> +++ b/drivers/target/iscsi/iscsi_target_auth.c
> @@ -302,11 +302,11 @@ static int chap_server_compute_md5(
> goto out;
> }
>
> + /* FIXME: What happens when simple_strtoul() return 256, 257, etc.? */
> if (type == HEX)
> - id = (unsigned char)simple_strtoul((char *)&identifier[2],
> - &endptr, 0);
> + id = simple_strtoul(&identifier[2], &endptr, 0);
> else
> - id = (unsigned char)simple_strtoul(identifier, &endptr, 0);
> + id = simple_strtoul(identifier, &endptr, 0);
> /*
> * RFC 1994 says Identifier is no more than octet (8 bits).
> */
Good catch.
Changing this to unsigned long usage with simple_strtoul, and adding a
explict check for an identifier above 255 as per RFC-1994.
> @@ -821,6 +820,7 @@ int iscsi_target_setup_login_socket(
> /*
> * Set SO_REUSEADDR, and disable Nagel Algorithm with TCP_NODELAY.
> */
> + /* FIXME: Someone please explain why this is endian-safe */
> opt = 1;
> if (np->np_network_transport == ISCSI_TCP) {
> ret = kernel_setsockopt(sock, IPPROTO_TCP, TCP_NODELAY,
> @@ -832,6 +832,7 @@ int iscsi_target_setup_login_socket(
> }
> }
>
> + /* FIXME: Someone please explain why this is endian-safe */
> ret = kernel_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
> (char *)&opt, sizeof(opt));
> if (ret < 0) {
Another good catch. Changing opt to be a single char value with
kernel_setsockopt().
Thanks Joern!
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
