Linux IIO development
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Justin Stitt <justinstitt@google.com>,
	Jonathan Cameron <jic23@kernel.org>,
	 Lars-Peter Clausen <lars@metafoo.de>
Cc: Stephen Boyd <swboyd@chromium.org>,
	linux-iio@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v3] iio: sx9324: avoid copying property strings
Date: Mon, 11 Dec 2023 22:30:12 -0800	[thread overview]
Message-ID: <f03d372a282712dee8412e47aff9bb54f181efd9.camel@perches.com> (raw)
In-Reply-To: <20231212-strncpy-drivers-iio-proximity-sx9324-c-v3-1-b8ae12fc8a5d@google.com>

On Tue, 2023-12-12 at 00:42 +0000, Justin Stitt wrote:
> We're doing some needless string copies when trying to assign the proper
> `prop` string. We can make `prop` a const char* and simply assign to
> string literals.

trivia:

I would have updated it like this moving the
various declarations into the case blocks
where they are used and removing a few unused
#defines

---
 drivers/iio/proximity/sx9324.c | 69 +++++++++++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/proximity/sx9324.c b/drivers/iio/proximity/sx9324.c
index ac2ed2da21ccc..c50c1108a69cc 100644
--- a/drivers/iio/proximity/sx9324.c
+++ b/drivers/iio/proximity/sx9324.c
@@ -877,17 +877,8 @@ static const struct sx_common_reg_default *
 sx9324_get_default_reg(struct device *dev, int idx,
 		       struct sx_common_reg_default *reg_def)
 {
-	static const char * const sx9324_rints[] = { "lowest", "low", "high",
-		"highest" };
-	static const char * const sx9324_csidle[] = { "hi-z", "hi-z", "gnd",
-		"vdd" };
-#define SX9324_PIN_DEF "semtech,ph0-pin"
-#define SX9324_RESOLUTION_DEF "semtech,ph01-resolution"
-#define SX9324_PROXRAW_DEF "semtech,ph01-proxraw-strength"
-	unsigned int pin_defs[SX9324_NUM_PINS];
-	char prop[] = SX9324_PROXRAW_DEF;
-	u32 start = 0, raw = 0, pos = 0;
-	int ret, count, ph, pin;
+	u32 raw = 0;
+	int ret;
 
 	memcpy(reg_def, &sx9324_default_regs[idx], sizeof(*reg_def));
 
@@ -896,7 +887,13 @@ sx9324_get_default_reg(struct device *dev, int idx,
 	case SX9324_REG_AFE_PH0:
 	case SX9324_REG_AFE_PH1:
 	case SX9324_REG_AFE_PH2:
-	case SX9324_REG_AFE_PH3:
+	case SX9324_REG_AFE_PH3: {
+		unsigned int pin_defs[SX9324_NUM_PINS];
+		int count;
+		int pin;
+		int ph;
+		char prop[32];
+
 		ph = reg_def->reg - SX9324_REG_AFE_PH0;
 		snprintf(prop, ARRAY_SIZE(prop), "semtech,ph%d-pin", ph);
 
@@ -913,7 +910,15 @@ sx9324_get_default_reg(struct device *dev, int idx,
 			       SX9324_REG_AFE_PH0_PIN_MASK(pin);
 		reg_def->def = raw;
 		break;
-	case SX9324_REG_AFE_CTRL0:
+	}
+	case SX9324_REG_AFE_CTRL0: {
+		static const char * const sx9324_csidle[] = {
+			"hi-z", "hi-z", "gnd", "vdd"
+		};
+		static const char * const sx9324_rints[] = {
+			"lowest", "low", "high", "highest"
+		};
+
 		ret = device_property_match_property_string(dev, "semtech,cs-idle-sleep",
 							    sx9324_csidle,
 							    ARRAY_SIZE(sx9324_csidle));
@@ -930,16 +935,17 @@ sx9324_get_default_reg(struct device *dev, int idx,
 			reg_def->def |= ret << SX9324_REG_AFE_CTRL0_RINT_SHIFT;
 		}
 		break;
+	}
 	case SX9324_REG_AFE_CTRL4:
-	case SX9324_REG_AFE_CTRL7:
+	case SX9324_REG_AFE_CTRL7: {
+		const char *type;
+
 		if (reg_def->reg == SX9324_REG_AFE_CTRL4)
-			strncpy(prop, "semtech,ph01-resolution",
-				ARRAY_SIZE(prop));
+			type = "semtech,ph01-resolution";
 		else
-			strncpy(prop, "semtech,ph23-resolution",
-				ARRAY_SIZE(prop));
+			type = "semtech,ph23-resolution";
 
-		ret = device_property_read_u32(dev, prop, &raw);
+		ret = device_property_read_u32(dev, type, &raw);
 		if (ret)
 			break;
 
@@ -949,6 +955,7 @@ sx9324_get_default_reg(struct device *dev, int idx,
 		reg_def->def |= FIELD_PREP(SX9324_REG_AFE_CTRL4_RESOLUTION_MASK,
 					   raw);
 		break;
+	}
 	case SX9324_REG_AFE_CTRL8:
 		ret = device_property_read_u32(dev,
 				"semtech,input-precharge-resistor-ohms",
@@ -982,17 +989,21 @@ sx9324_get_default_reg(struct device *dev, int idx,
 					   6 + raw * (raw + 3) / 2);
 		break;
 
-	case SX9324_REG_ADV_CTRL5:
+	case SX9324_REG_ADV_CTRL5: {
+		u32 start = 0;
+
 		ret = device_property_read_u32(dev, "semtech,startup-sensor",
 					       &start);
 		if (ret)
 			break;
-
 		reg_def->def &= ~SX9324_REG_ADV_CTRL5_STARTUPSENS_MASK;
 		reg_def->def |= FIELD_PREP(SX9324_REG_ADV_CTRL5_STARTUPSENS_MASK,
 					   start);
 		break;
-	case SX9324_REG_PROX_CTRL4:
+	}
+	case SX9324_REG_PROX_CTRL4: {
+		u32 pos = 0;
+
 		ret = device_property_read_u32(dev, "semtech,avg-pos-strength",
 					       &pos);
 		if (ret)
@@ -1005,15 +1016,16 @@ sx9324_get_default_reg(struct device *dev, int idx,
 		reg_def->def |= FIELD_PREP(SX9324_REG_PROX_CTRL4_AVGPOSFILT_MASK,
 					   raw);
 		break;
+	}
 	case SX9324_REG_PROX_CTRL0:
-	case SX9324_REG_PROX_CTRL1:
+	case SX9324_REG_PROX_CTRL1: {
+		const char *type;
+
 		if (reg_def->reg == SX9324_REG_PROX_CTRL0)
-			strncpy(prop, "semtech,ph01-proxraw-strength",
-				ARRAY_SIZE(prop));
+			type = "semtech,ph01-proxraw-strength";
 		else
-			strncpy(prop, "semtech,ph23-proxraw-strength",
-				ARRAY_SIZE(prop));
-		ret = device_property_read_u32(dev, prop, &raw);
+			type = "semtech,ph23-proxraw-strength";
+		ret = device_property_read_u32(dev, type, &raw);
 		if (ret)
 			break;
 
@@ -1022,6 +1034,7 @@ sx9324_get_default_reg(struct device *dev, int idx,
 					   raw);
 		break;
 	}
+	}
 	return reg_def;
 }
 


  reply	other threads:[~2023-12-12  6:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12  0:42 [PATCH v3] iio: sx9324: avoid copying property strings Justin Stitt
2023-12-12  6:30 ` Joe Perches [this message]
2023-12-17 13:24   ` Jonathan Cameron
2023-12-17 13:41     ` Joe Perches
2023-12-12 21:26 ` Kees Cook
2023-12-14  0:59 ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f03d372a282712dee8412e47aff9bb54f181efd9.camel@perches.com \
    --to=joe@perches.com \
    --cc=jic23@kernel.org \
    --cc=justinstitt@google.com \
    --cc=lars@metafoo.de \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox