[PATCH v2 2/2] usb: gadget: printer: Remove pnp_string static buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



pnp string is usually much shorter than 1k so let's stop wasting 1k of
memory for its buffer and make it dynamically alocated.
This also removes 1k len limitation for pnp_string and
adds a new line after string content if required.

Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
---
Changes since v1:
- assignt true instead of statement with no effect (reported by kbuild)
---
 drivers/usb/gadget/function/f_printer.c | 57 +++++++++++++++++++++++++--------
 drivers/usb/gadget/function/u_printer.h |  5 ++-
 drivers/usb/gadget/legacy/printer.c     | 28 +++++++++-------
 3 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 0de36cda6e41..36fe181e72c5 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -49,7 +49,6 @@
 
 #include "u_printer.h"
 
-#define PNP_STRING_LEN		1024
 #define PRINTER_MINORS		4
 #define GET_DEVICE_ID		0
 #define GET_PORT_STATUS		1
@@ -907,8 +906,7 @@ static bool gprinter_req_match(struct usb_function *f,
 	switch (ctrl->bRequest) {
 	case GET_DEVICE_ID:
 		w_index >>= 8;
-		if (w_length <= PNP_STRING_LEN &&
-		    (USB_DIR_IN & ctrl->bRequestType))
+		if (USB_DIR_IN & ctrl->bRequestType)
 			break;
 		return false;
 	case GET_PORT_STATUS:
@@ -937,6 +935,7 @@ static int printer_func_setup(struct usb_function *f,
 	struct printer_dev *dev = func_to_printer(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct usb_request	*req = cdev->req;
+	u8			*buf = req->buf;
 	int			value = -EOPNOTSUPP;
 	u16			wIndex = le16_to_cpu(ctrl->wIndex);
 	u16			wValue = le16_to_cpu(ctrl->wValue);
@@ -953,10 +952,16 @@ static int printer_func_setup(struct usb_function *f,
 			if ((wIndex>>8) != dev->interface)
 				break;
 
-			value = (dev->pnp_string[0] << 8) | dev->pnp_string[1];
-			memcpy(req->buf, dev->pnp_string, value);
+			if (!dev->pnp_string) {
+				value = 0;
+				break;
+			}
+			value = strlen(dev->pnp_string);
+			buf[0] = (value >> 8) & 0xFF;
+			buf[1] = value & 0xFF;
+			memcpy(buf + 2, dev->pnp_string, value);
 			DBG(dev, "1284 PNP String: %x %s\n", value,
-					&dev->pnp_string[2]);
+			    dev->pnp_string);
 			break;
 
 		case GET_PORT_STATUS: /* Get Port Status */
@@ -964,7 +969,7 @@ static int printer_func_setup(struct usb_function *f,
 			if (wIndex != dev->interface)
 				break;
 
-			*(u8 *)req->buf = dev->printer_status;
+			buf[0] = dev->printer_status;
 			value = min_t(u16, wLength, 1);
 			break;
 
@@ -1157,10 +1162,21 @@ static ssize_t f_printer_opts_pnp_string_show(struct config_item *item,
 					      char *page)
 {
 	struct f_printer_opts *opts = to_f_printer_opts(item);
-	int result;
+	int result = 0;
 
 	mutex_lock(&opts->lock);
-	result = strlcpy(page, opts->pnp_string + 2, PNP_STRING_LEN - 2);
+	if (!opts->pnp_string)
+		goto unlock;
+
+	result = strlcpy(page, opts->pnp_string, PAGE_SIZE);
+	if (result >= PAGE_SIZE) {
+		result = PAGE_SIZE;
+	} else if (page[result - 1] != '\n' && result + 1 < PAGE_SIZE) {
+		page[result++] = '\n';
+		page[result] = '\0';
+	}
+
+unlock:
 	mutex_unlock(&opts->lock);
 
 	return result;
@@ -1170,13 +1186,24 @@ static ssize_t f_printer_opts_pnp_string_store(struct config_item *item,
 					       const char *page, size_t len)
 {
 	struct f_printer_opts *opts = to_f_printer_opts(item);
-	int result, l;
+	char *new_pnp;
+	int result;
 
 	mutex_lock(&opts->lock);
-	result = strlcpy(opts->pnp_string + 2, page, PNP_STRING_LEN - 2);
-	l = strlen(opts->pnp_string + 2) + 2;
-	opts->pnp_string[0] = (l >> 8) & 0xFF;
-	opts->pnp_string[1] = l & 0xFF;
+
+	new_pnp = kstrndup(page, len, GFP_KERNEL);
+	if (!new_pnp) {
+		result = -ENOMEM;
+		goto unlock;
+	}
+
+	if (opts->pnp_string_allocated)
+		kfree(opts->pnp_string);
+
+	opts->pnp_string_allocated = true;
+	opts->pnp_string = new_pnp;
+	result = len;
+unlock:
 	mutex_unlock(&opts->lock);
 
 	return result;
@@ -1270,6 +1297,8 @@ static void gprinter_free_inst(struct usb_function_instance *f)
 
 	mutex_unlock(&printer_ida_lock);
 
+	if (opts->pnp_string_allocated)
+		kfree(opts->pnp_string);
 	kfree(opts);
 }
 
diff --git a/drivers/usb/gadget/function/u_printer.h b/drivers/usb/gadget/function/u_printer.h
index 0e2c49d4274e..8d30b7577f87 100644
--- a/drivers/usb/gadget/function/u_printer.h
+++ b/drivers/usb/gadget/function/u_printer.h
@@ -18,12 +18,11 @@
 
 #include <linux/usb/composite.h>
 
-#define PNP_STRING_LEN			1024
-
 struct f_printer_opts {
 	struct usb_function_instance	func_inst;
 	int				minor;
-	char				pnp_string[PNP_STRING_LEN];
+	char				*pnp_string;
+	bool				pnp_string_allocated;
 	unsigned			q_len;
 
 	/*
diff --git a/drivers/usb/gadget/legacy/printer.c b/drivers/usb/gadget/legacy/printer.c
index 6f969a86175c..4c9cfff34a03 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -88,8 +88,8 @@ static const struct usb_descriptor_header *otg_desc[2];
 
 static char				product_desc [40] = DRIVER_DESC;
 static char				serial_num [40] = "1";
-static char				pnp_string[PNP_STRING_LEN] =
-	"XXMFG:linux;MDL:g_printer;CLS:PRINTER;SN:1;";
+static char				*pnp_string =
+	"MFG:linux;MDL:g_printer;CLS:PRINTER;SN:1;";
 
 /* static strings, in UTF-8 */
 static struct usb_string		strings [] = {
@@ -143,23 +143,29 @@ static int printer_do_config(struct usb_configuration *c)
 static int printer_bind(struct usb_composite_dev *cdev)
 {
 	struct f_printer_opts *opts;
-	int ret, len;
+	int ret;
 
 	fi_printer = usb_get_function_instance("printer");
 	if (IS_ERR(fi_printer))
 		return PTR_ERR(fi_printer);
 
-	if (iPNPstring)
-		strlcpy(&pnp_string[2], iPNPstring, PNP_STRING_LEN - 2);
-
-	len = strlen(pnp_string);
-	pnp_string[0] = (len >> 8) & 0xFF;
-	pnp_string[1] = len & 0xFF;
-
 	opts = container_of(fi_printer, struct f_printer_opts, func_inst);
 	opts->minor = 0;
-	memcpy(opts->pnp_string, pnp_string, PNP_STRING_LEN);
 	opts->q_len = QLEN;
+	if (iPNPstring) {
+		opts->pnp_string = kstrdup(iPNPstring, GFP_KERNEL);
+		if (!opts->pnp_string) {
+			ret = -ENOMEM;
+			goto fail_put_func_inst;
+		}
+		opts->pnp_string_allocated = true;
+		/*
+		 * we don't free this memory in case of error
+		 * as printer cleanup func will do this for us
+		 */
+	} else {
+		opts->pnp_string = pnp_string;
+	}
 
 	ret = usb_string_ids_tab(cdev, strings);
 	if (ret < 0)
-- 
2.9.3

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux