isdn/gigaset: restructure modem response parser

PATCH In review Tilman Schmidt 2 participants 6 messages Mar 21, 2015 – Mar 23, 2015

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Tilman Schmidt

author

2015-03-21 19:49 UTC #0 lore raw

cover isdn/gigaset: restructure modem response parser

This series of patches restructures the Gigaset ISDN driver's modem response parser to improve code readability and conform better to the device's specification and actual behaviour. Could you please merge these through net-next?

Thanks,
Tilman

Tilman Schmidt (4):
isdn/gigaset: restructure modem response parser (1) isdn/gigaset: restructure modem response parser (2) isdn/gigaset: restructure modem response parser (3) isdn/gigaset: restructure modem response parser (4)

 drivers/isdn/gigaset/ev-layer.c | 365 +++++++++++++++++++++-------------------
 1 file changed, 195 insertions(+), 170 deletions(-)

Signature · 1 line

1.9.2.459.g68773ac

Tilman Schmidt

author

2015-03-21 19:50 UTC #3 lore raw

PATCH 3/4 isdn/gigaset: restructure modem response parser (3)

Separate CID detection from main parser loop.

Signed-off-by Tilman Schmidt <[email protected]>

---
 drivers/isdn/gigaset/ev-layer.c | 56 ++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 37 deletions(-)

drivers/isdn/gigaset/ev-layer.c +19 ~~-37~~

index b2a1fb1..dc6e6a3 100644
@@ -389,24 +389,6 @@ zsau_resp[] =
 	{NULL,				ZSAU_UNKNOWN}
 };
 
-/* retrieve CID from parsed response
- * returns 0 if no CID, -1 if invalid CID, or CID value 1..65535
- */
-static int cid_of_response(char *s)
-{
-	int cid;
-	int rc;
-
-	if (s[-1] != ';')
-		return 0;	/* no CID separator */
-	rc = kstrtoint(s, 10, &cid);
-	if (rc)
-		return 0;	/* CID not numeric */
-	if (cid < 1 || cid > 65535)
-		return -1;	/* CID out of range */
-	return cid;
-}
-
 /* queue event with CID */
 static void add_cid_event(struct cardstate *cs, int cid, int type,
 			  void *ptr, int parameter)

@@ -451,7 +433,7 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 	unsigned char *argv[MAX_REC_PARAMS + 1];
 	int params;
 	int i, j;
-	char *ptr;
+	char *psep, *ptr;
 	const struct resp_type_t *rt;
 	const struct zsau_resp_t *zr;
 	int curarg;

@@ -474,6 +456,18 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 		return;
 	}
 
+	/* check for CID */
+	psep = strrchr(cs->respdata, ';');
+	if (psep &&
+	    !kstrtoint(psep + 1, 10, &cid) &&
+	    cid >= 1 && cid <= 65535) {
+		/* valid CID: chop it off */
+		*psep = 0;
+	} else {
+		/* no valid CID: leave unchanged */
+		cid = 0;
+	}
+
 	/* parse line */
 	argv[0] = cs->respdata;
 	params = 1;

@@ -482,29 +476,17 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 		case ';':
 		case ',':
 		case '=':
-			if (params > MAX_REC_PARAMS) {
+			cs->respdata[i] = 0;
+			if (params > MAX_REC_PARAMS)
 				dev_warn(cs->dev,
 					 "too many parameters in response\n");
-				/* need last parameter (might be CID) */
-				params--;
-			}
-			argv[params++] = cs->respdata + i + 1;
+			else
+				argv[params++] = cs->respdata + i + 1;
 		}
 
-	cid = params > 1 ? cid_of_response(argv[params - 1]) : 0;
-	if (cid < 0) {
-		gigaset_add_event(cs, &cs->at_state, RSP_INVAL, NULL, 0, NULL);
-		return;
-	}
-
-	for (j = 1; j < params; ++j)
-		argv[j][-1] = 0;
-
 	gig_dbg(DEBUG_EVENT, "CMD received: %s", argv[0]);
-	if (cid) {
-		--params;
-		gig_dbg(DEBUG_EVENT, "CID: %s", argv[params]);
-	}
+	if (cid)
+		gig_dbg(DEBUG_EVENT, "CID: %d", cid);
 	gig_dbg(DEBUG_EVENT, "available params: %d", params - 1);
 	for (j = 1; j < params; j++)
 		gig_dbg(DEBUG_EVENT, "param %d: %s", j, argv[j]);

Signature · 1 line

1.9.2.459.g68773ac

Tilman Schmidt

author

2015-03-21 19:50 UTC #1 lore raw

PATCH 1/4 isdn/gigaset: restructure modem response parser (1)

In reply to Tilman Schmidt

Factor out queueing of modem response events into helper function add_cid_event().

Signed-off-by Tilman Schmidt <[email protected]>

---
 drivers/isdn/gigaset/ev-layer.c | 99 +++++++++++++++++++++++------------------
 1 file changed, 55 insertions(+), 44 deletions(-)

drivers/isdn/gigaset/ev-layer.c +55 ~~-44~~

index c8ced12..d1b84f4 100644
@@ -407,6 +407,37 @@ static int cid_of_response(char *s)
 	return cid;
 }
 
+/* queue event with CID */
+static void add_cid_event(struct cardstate *cs, int cid, int type,
+			  void *ptr, int parameter)
+{
+	unsigned long flags;
+	unsigned next, tail;
+	struct event_t *event;
+
+	gig_dbg(DEBUG_EVENT, "queueing event %d for cid %d", type, cid);
+
+	spin_lock_irqsave(&cs->ev_lock, flags);
+
+	tail = cs->ev_tail;
+	next = (tail + 1) % MAX_EVENTS;
+	if (unlikely(next == cs->ev_head)) {
+		dev_err(cs->dev, "event queue full\n");
+		kfree(ptr);
+	} else {
+		event = cs->events + tail;
+		event->type = type;
+		event->cid = cid;
+		event->ptr = ptr;
+		event->arg = NULL;
+		event->parameter = parameter;
+		event->at_state = NULL;
+		cs->ev_tail = next;
+	}
+
+	spin_unlock_irqrestore(&cs->ev_lock, flags);
+}
+
 /**
  * gigaset_handle_modem_response() - process received modem response
  * @cs:		device descriptor structure.

@@ -420,17 +451,15 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 	unsigned char *argv[MAX_REC_PARAMS + 1];
 	int params;
 	int i, j;
+	char *ptr;
 	const struct resp_type_t *rt;
 	const struct zsau_resp_t *zr;
 	int curarg;
-	unsigned long flags;
-	unsigned next, tail, head;
-	struct event_t *event;
 	int resp_code;
 	int param_type;
 	int abort;
 	size_t len;
-	int cid;
+	int cid, parameter;
 	int rawstring;
 
 	len = cs->cbytes;

@@ -484,26 +513,9 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 			gig_dbg(DEBUG_EVENT, "param %d: %s", j, argv[j]);
 	}
 
-	spin_lock_irqsave(&cs->ev_lock, flags);
-	head = cs->ev_head;
-	tail = cs->ev_tail;
-
 	abort = 1;
 	curarg = 0;
 	while (curarg < params) {
-		next = (tail + 1) % MAX_EVENTS;
-		if (unlikely(next == head)) {
-			dev_err(cs->dev, "event queue full\n");
-			break;
-		}
-
-		event = cs->events + tail;
-		event->at_state = NULL;
-		event->cid = cid;
-		event->ptr = NULL;
-		event->arg = NULL;
-		tail = next;
-
 		if (rawstring) {
 			resp_code = RSP_STRING;
 			param_type = RT_STRING;

@@ -513,7 +525,7 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 					break;
 
 			if (!rt->response) {
-				event->type = RSP_NONE;
+				add_cid_event(cs, 0, RSP_NONE, NULL, 0);
 				gig_dbg(DEBUG_EVENT,
 					"unknown modem response: '%s'\n",
 					argv[curarg]);

@@ -525,78 +537,77 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 			++curarg;
 		}
 
-		event->type = resp_code;
-
 		switch (param_type) {
 		case RT_NOTHING:
+			add_cid_event(cs, cid, resp_code, NULL, 0);
 			break;
 		case RT_RING:
 			if (!cid) {
 				dev_err(cs->dev,
 					"received RING without CID!\n");
-				event->type = RSP_INVAL;
+				add_cid_event(cs, 0, RSP_INVAL, NULL, 0);
 				abort = 1;
 			} else {
-				event->cid = 0;
-				event->parameter = cid;
+				add_cid_event(cs, 0, resp_code, NULL, cid);
 				abort = 0;
 			}
 			break;
 		case RT_ZSAU:
 			if (curarg >= params) {
-				event->parameter = ZSAU_NONE;
+				add_cid_event(cs, cid, resp_code, NULL,
+					      ZSAU_NONE);
 				break;
 			}
 			for (zr = zsau_resp; zr->str; ++zr)
 				if (!strcmp(argv[curarg], zr->str))
 					break;
-			event->parameter = zr->code;
 			if (!zr->str)
 				dev_warn(cs->dev,
 					 "%s: unknown parameter %s after ZSAU\n",
 					 __func__, argv[curarg]);
+			add_cid_event(cs, cid, resp_code, NULL, zr->code);
 			++curarg;
 			break;
 		case RT_STRING:
 			if (curarg < params) {
-				event->ptr = kstrdup(argv[curarg], GFP_ATOMIC);
-				if (!event->ptr)
+				ptr = kstrdup(argv[curarg], GFP_ATOMIC);
+				if (!ptr)
 					dev_err(cs->dev, "out of memory\n");
 				++curarg;
+			} else {
+				ptr = NULL;
 			}
-			gig_dbg(DEBUG_EVENT, "string==%s",
-				event->ptr ? (char *) event->ptr : "NULL");
+			gig_dbg(DEBUG_EVENT, "string==%s", ptr ? ptr : "NULL");
+			add_cid_event(cs, cid, resp_code, ptr, 0);
 			break;
 		case RT_ZCAU:
-			event->parameter = -1;
+			parameter = -1;
 			if (curarg + 1 < params) {
 				u8 type, value;
 
 				i = kstrtou8(argv[curarg++], 16, &type);
 				j = kstrtou8(argv[curarg++], 16, &value);
 				if (i == 0 && j == 0)
-					event->parameter = (type << 8) | value;
+					parameter = (type << 8) | value;
 			} else
 				curarg = params - 1;
+			add_cid_event(cs, cid, resp_code, NULL, parameter);
 			break;
 		case RT_NUMBER:
 			if (curarg >= params ||
-			    kstrtoint(argv[curarg++], 10, &event->parameter))
-				event->parameter = -1;
-			gig_dbg(DEBUG_EVENT, "parameter==%d", event->parameter);
+			    kstrtoint(argv[curarg++], 10, &parameter))
+				parameter = -1;
+			gig_dbg(DEBUG_EVENT, "parameter==%d", parameter);
+			add_cid_event(cs, cid, resp_code, NULL, parameter);
+			if (resp_code == RSP_ZDLE)
+				cs->dle = parameter;
 			break;
 		}
 
-		if (resp_code == RSP_ZDLE)
-			cs->dle = event->parameter;
-
 		if (abort)
 			break;
 	}
 
-	cs->ev_tail = tail;
-	spin_unlock_irqrestore(&cs->ev_lock, flags);
-
 	if (curarg != params)
 		gig_dbg(DEBUG_EVENT,
 			"invalid number of processed parameters: %d/%d",

Signature · 1 line

1.9.2.459.g68773ac

Tilman Schmidt

author

2015-03-21 19:50 UTC #2 lore raw

PATCH 2/4 isdn/gigaset: restructure modem response parser (2)

In reply to Tilman Schmidt

Separate literal string handling from main parser loop.

Signed-off-by Tilman Schmidt <[email protected]>

---
 drivers/isdn/gigaset/ev-layer.c | 126 ++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 69 deletions(-)

drivers/isdn/gigaset/ev-layer.c +57 ~~-69~~

index d1b84f4..b2a1fb1 100644
@@ -455,91 +455,79 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 	const struct resp_type_t *rt;
 	const struct zsau_resp_t *zr;
 	int curarg;
-	int resp_code;
-	int param_type;
 	int abort;
-	size_t len;
 	int cid, parameter;
-	int rawstring;
 
-	len = cs->cbytes;
-	if (!len) {
+	if (!cs->cbytes) {
 		/* ignore additional LFs/CRs (M10x config mode or cx100) */
 		gig_dbg(DEBUG_MCMD, "skipped EOL [%02X]", cs->respdata[0]);
 		return;
 	}
-	cs->respdata[len] = 0;
-	argv[0] = cs->respdata;
-	params = 1;
+	cs->respdata[cs->cbytes] = 0;
+
 	if (cs->at_state.getstring) {
-		/* getstring only allowed without cid at the moment */
+		/* state machine wants next line verbatim */
 		cs->at_state.getstring = 0;
-		rawstring = 1;
-		cid = 0;
-	} else {
-		/* parse line */
-		for (i = 0; i < len; i++)
-			switch (cs->respdata[i]) {
-			case ';':
-			case ',':
-			case '=':
-				if (params > MAX_REC_PARAMS) {
-					dev_warn(cs->dev,
-						 "too many parameters in response\n");
-					/* need last parameter (might be CID) */
-					params--;
-				}
-				argv[params++] = cs->respdata + i + 1;
-			}
+		ptr = kstrdup(cs->respdata, GFP_ATOMIC);
+		gig_dbg(DEBUG_EVENT, "string==%s", ptr ? ptr : "NULL");
+		add_cid_event(cs, 0, RSP_STRING, ptr, 0);
+		return;
+	}
 
-		rawstring = 0;
-		cid = params > 1 ? cid_of_response(argv[params - 1]) : 0;
-		if (cid < 0) {
-			gigaset_add_event(cs, &cs->at_state, RSP_INVAL,
-					  NULL, 0, NULL);
-			return;
+	/* parse line */
+	argv[0] = cs->respdata;
+	params = 1;
+	for (i = 0; i < cs->cbytes; i++)
+		switch (cs->respdata[i]) {
+		case ';':
+		case ',':
+		case '=':
+			if (params > MAX_REC_PARAMS) {
+				dev_warn(cs->dev,
+					 "too many parameters in response\n");
+				/* need last parameter (might be CID) */
+				params--;
+			}
+			argv[params++] = cs->respdata + i + 1;
 		}
 
-		for (j = 1; j < params; ++j)
-			argv[j][-1] = 0;
+	cid = params > 1 ? cid_of_response(argv[params - 1]) : 0;
+	if (cid < 0) {
+		gigaset_add_event(cs, &cs->at_state, RSP_INVAL, NULL, 0, NULL);
+		return;
+	}
+
+	for (j = 1; j < params; ++j)
+		argv[j][-1] = 0;
 
-		gig_dbg(DEBUG_EVENT, "CMD received: %s", argv[0]);
-		if (cid) {
-			--params;
-			gig_dbg(DEBUG_EVENT, "CID: %s", argv[params]);
-		}
-		gig_dbg(DEBUG_EVENT, "available params: %d", params - 1);
-		for (j = 1; j < params; j++)
-			gig_dbg(DEBUG_EVENT, "param %d: %s", j, argv[j]);
+	gig_dbg(DEBUG_EVENT, "CMD received: %s", argv[0]);
+	if (cid) {
+		--params;
+		gig_dbg(DEBUG_EVENT, "CID: %s", argv[params]);
 	}
+	gig_dbg(DEBUG_EVENT, "available params: %d", params - 1);
+	for (j = 1; j < params; j++)
+		gig_dbg(DEBUG_EVENT, "param %d: %s", j, argv[j]);
 
 	abort = 1;
 	curarg = 0;
 	while (curarg < params) {
-		if (rawstring) {
-			resp_code = RSP_STRING;
-			param_type = RT_STRING;
-		} else {
-			for (rt = resp_type; rt->response; ++rt)
-				if (!strcmp(argv[curarg], rt->response))
-					break;
-
-			if (!rt->response) {
-				add_cid_event(cs, 0, RSP_NONE, NULL, 0);
-				gig_dbg(DEBUG_EVENT,
-					"unknown modem response: '%s'\n",
-					argv[curarg]);
+		for (rt = resp_type; rt->response; ++rt)
+			if (!strcmp(argv[curarg], rt->response))
 				break;
-			}
 
-			resp_code = rt->resp_code;
-			param_type = rt->type;
-			++curarg;
+		if (!rt->response) {
+			add_cid_event(cs, 0, RSP_NONE, NULL, 0);
+			gig_dbg(DEBUG_EVENT, "unknown modem response: '%s'\n",
+				argv[curarg]);
+			break;
 		}
 
-		switch (param_type) {
+		++curarg;
+
+		switch (rt->type) {
 		case RT_NOTHING:
-			add_cid_event(cs, cid, resp_code, NULL, 0);
+			add_cid_event(cs, cid, rt->resp_code, NULL, 0);
 			break;
 		case RT_RING:
 			if (!cid) {

@@ -548,13 +536,13 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 				add_cid_event(cs, 0, RSP_INVAL, NULL, 0);
 				abort = 1;
 			} else {
-				add_cid_event(cs, 0, resp_code, NULL, cid);
+				add_cid_event(cs, 0, rt->resp_code, NULL, cid);
 				abort = 0;
 			}
 			break;
 		case RT_ZSAU:
 			if (curarg >= params) {
-				add_cid_event(cs, cid, resp_code, NULL,
+				add_cid_event(cs, cid, rt->resp_code, NULL,
 					      ZSAU_NONE);
 				break;
 			}

@@ -565,7 +553,7 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 				dev_warn(cs->dev,
 					 "%s: unknown parameter %s after ZSAU\n",
 					 __func__, argv[curarg]);
-			add_cid_event(cs, cid, resp_code, NULL, zr->code);
+			add_cid_event(cs, cid, rt->resp_code, NULL, zr->code);
 			++curarg;
 			break;
 		case RT_STRING:

@@ -578,7 +566,7 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 				ptr = NULL;
 			}
 			gig_dbg(DEBUG_EVENT, "string==%s", ptr ? ptr : "NULL");
-			add_cid_event(cs, cid, resp_code, ptr, 0);
+			add_cid_event(cs, cid, rt->resp_code, ptr, 0);
 			break;
 		case RT_ZCAU:
 			parameter = -1;

@@ -591,15 +579,15 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 					parameter = (type << 8) | value;
 			} else
 				curarg = params - 1;
-			add_cid_event(cs, cid, resp_code, NULL, parameter);
+			add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
 			break;
 		case RT_NUMBER:
 			if (curarg >= params ||
 			    kstrtoint(argv[curarg++], 10, &parameter))
 				parameter = -1;
 			gig_dbg(DEBUG_EVENT, "parameter==%d", parameter);
-			add_cid_event(cs, cid, resp_code, NULL, parameter);
-			if (resp_code == RSP_ZDLE)
+			add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
+			if (rt->resp_code == RSP_ZDLE)
 				cs->dle = parameter;
 			break;
 		}

Signature · 1 line

1.9.2.459.g68773ac

Tilman Schmidt

author

2015-03-21 19:50 UTC #4 lore raw

PATCH 4/4 isdn/gigaset: restructure modem response parser (4)

In reply to Tilman Schmidt

Restructure the control structure of the modem response parser to improve readability and error handling.

Signed-off-by Tilman Schmidt <[email protected]>

---
 drivers/isdn/gigaset/ev-layer.c | 258 +++++++++++++++++++++++-----------------
 1 file changed, 151 insertions(+), 107 deletions(-)

drivers/isdn/gigaset/ev-layer.c +151 ~~-107~~

index dc6e6a3..1cfcea6 100644
@@ -389,6 +389,20 @@ zsau_resp[] =
 	{NULL,				ZSAU_UNKNOWN}
 };
 
+/* check for and remove fixed string prefix
+ * If s starts with prefix terminated by a non-alphanumeric character,
+ * return pointer to the first character after that, otherwise return NULL.
+ */
+static char *skip_prefix(char *s, const char *prefix)
+{
+	while (*prefix)
+		if (*s++ != *prefix++)
+			return NULL;
+	if (isalnum(*s))
+		return NULL;
+	return s;
+}
+
 /* queue event with CID */
 static void add_cid_event(struct cardstate *cs, int cid, int type,
 			  void *ptr, int parameter)

@@ -430,15 +444,11 @@ static void add_cid_event(struct cardstate *cs, int cid, int type,
  */
 void gigaset_handle_modem_response(struct cardstate *cs)
 {
-	unsigned char *argv[MAX_REC_PARAMS + 1];
-	int params;
-	int i, j;
-	char *psep, *ptr;
+	char *eoc, *psep, *ptr;
 	const struct resp_type_t *rt;
 	const struct zsau_resp_t *zr;
-	int curarg;
-	int abort;
 	int cid, parameter;
+	u8 type, value;
 
 	if (!cs->cbytes) {
 		/* ignore additional LFs/CRs (M10x config mode or cx100) */

@@ -456,6 +466,19 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 		return;
 	}
 
+	/* look up response type */
+	for (rt = resp_type; rt->response; ++rt) {
+		eoc = skip_prefix(cs->respdata, rt->response);
+		if (eoc)
+			break;
+	}
+	if (!rt->response) {
+		add_cid_event(cs, 0, RSP_NONE, NULL, 0);
+		gig_dbg(DEBUG_EVENT, "unknown modem response: '%s'\n",
+			cs->respdata);
+		return;
+	}
+
 	/* check for CID */
 	psep = strrchr(cs->respdata, ';');
 	if (psep &&

@@ -468,120 +491,141 @@ void gigaset_handle_modem_response(struct cardstate *cs)
 		cid = 0;
 	}
 
-	/* parse line */
-	argv[0] = cs->respdata;
-	params = 1;
-	for (i = 0; i < cs->cbytes; i++)
-		switch (cs->respdata[i]) {
-		case ';':
-		case ',':
-		case '=':
-			cs->respdata[i] = 0;
-			if (params > MAX_REC_PARAMS)
-				dev_warn(cs->dev,
-					 "too many parameters in response\n");
-			else
-				argv[params++] = cs->respdata + i + 1;
-		}
-
-	gig_dbg(DEBUG_EVENT, "CMD received: %s", argv[0]);
+	gig_dbg(DEBUG_EVENT, "CMD received: %s", cs->respdata);
 	if (cid)
 		gig_dbg(DEBUG_EVENT, "CID: %d", cid);
-	gig_dbg(DEBUG_EVENT, "available params: %d", params - 1);
-	for (j = 1; j < params; j++)
-		gig_dbg(DEBUG_EVENT, "param %d: %s", j, argv[j]);
-
-	abort = 1;
-	curarg = 0;
-	while (curarg < params) {
-		for (rt = resp_type; rt->response; ++rt)
-			if (!strcmp(argv[curarg], rt->response))
-				break;
 
-		if (!rt->response) {
-			add_cid_event(cs, 0, RSP_NONE, NULL, 0);
-			gig_dbg(DEBUG_EVENT, "unknown modem response: '%s'\n",
-				argv[curarg]);
-			break;
-		}
+	switch (rt->type) {
+	case RT_NOTHING:
+		/* check parameter separator */
+		if (*eoc)
+			goto bad_param;	/* extra parameter */
 
-		++curarg;
+		add_cid_event(cs, cid, rt->resp_code, NULL, 0);
+		break;
 
-		switch (rt->type) {
-		case RT_NOTHING:
-			add_cid_event(cs, cid, rt->resp_code, NULL, 0);
-			break;
-		case RT_RING:
-			if (!cid) {
-				dev_err(cs->dev,
-					"received RING without CID!\n");
-				add_cid_event(cs, 0, RSP_INVAL, NULL, 0);
-				abort = 1;
-			} else {
-				add_cid_event(cs, 0, rt->resp_code, NULL, cid);
-				abort = 0;
-			}
-			break;
-		case RT_ZSAU:
-			if (curarg >= params) {
-				add_cid_event(cs, cid, rt->resp_code, NULL,
-					      ZSAU_NONE);
-				break;
-			}
-			for (zr = zsau_resp; zr->str; ++zr)
-				if (!strcmp(argv[curarg], zr->str))
+	case RT_RING:
+		/* check parameter separator */
+		if (!*eoc)
+			eoc = NULL;	/* no parameter */
+		else if (*eoc++ != ',')
+			goto bad_param;
+
+		add_cid_event(cs, 0, rt->resp_code, NULL, cid);
+
+		/* process parameters as individual responses */
+		while (eoc) {
+			/* look up parameter type */
+			psep = NULL;
+			for (rt = resp_type; rt->response; ++rt) {
+				psep = skip_prefix(eoc, rt->response);
+				if (psep)
 					break;
-			if (!zr->str)
+			}
+
+			/* all legal parameters are of type RT_STRING */
+			if (!psep || rt->type != RT_STRING) {
 				dev_warn(cs->dev,
-					 "%s: unknown parameter %s after ZSAU\n",
-					 __func__, argv[curarg]);
-			add_cid_event(cs, cid, rt->resp_code, NULL, zr->code);
-			++curarg;
-			break;
-		case RT_STRING:
-			if (curarg < params) {
-				ptr = kstrdup(argv[curarg], GFP_ATOMIC);
-				if (!ptr)
-					dev_err(cs->dev, "out of memory\n");
-				++curarg;
-			} else {
-				ptr = NULL;
+					 "illegal RING parameter: '%s'\n",
+					 eoc);
+				return;
 			}
-			gig_dbg(DEBUG_EVENT, "string==%s", ptr ? ptr : "NULL");
+
+			/* skip parameter value separator */
+			if (*psep++ != '=')
+				goto bad_param;
+
+			/* look up end of parameter */
+			eoc = strchr(psep, ',');
+			if (eoc)
+				*eoc++ = 0;
+
+			/* retrieve parameter value */
+			ptr = kstrdup(psep, GFP_ATOMIC);
+
+			/* queue event */
 			add_cid_event(cs, cid, rt->resp_code, ptr, 0);
-			break;
-		case RT_ZCAU:
-			parameter = -1;
-			if (curarg + 1 < params) {
-				u8 type, value;
-
-				i = kstrtou8(argv[curarg++], 16, &type);
-				j = kstrtou8(argv[curarg++], 16, &value);
-				if (i == 0 && j == 0)
-					parameter = (type << 8) | value;
-			} else
-				curarg = params - 1;
-			add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
-			break;
-		case RT_NUMBER:
-			if (curarg >= params ||
-			    kstrtoint(argv[curarg++], 10, &parameter))
-				parameter = -1;
-			gig_dbg(DEBUG_EVENT, "parameter==%d", parameter);
-			add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
-			if (rt->resp_code == RSP_ZDLE)
-				cs->dle = parameter;
-			break;
 		}
+		break;
 
-		if (abort)
+	case RT_ZSAU:
+		/* check parameter separator */
+		if (!*eoc) {
+			/* no parameter */
+			add_cid_event(cs, cid, rt->resp_code, NULL, ZSAU_NONE);
 			break;
-	}
+		}
+		if (*eoc++ != '=')
+			goto bad_param;
 
-	if (curarg != params)
-		gig_dbg(DEBUG_EVENT,
-			"invalid number of processed parameters: %d/%d",
-			curarg, params);
+		/* look up parameter value */
+		for (zr = zsau_resp; zr->str; ++zr)
+			if (!strcmp(eoc, zr->str))
+				break;
+		if (!zr->str)
+			goto bad_param;
+
+		add_cid_event(cs, cid, rt->resp_code, NULL, zr->code);
+		break;
+
+	case RT_STRING:
+		/* check parameter separator */
+		if (*eoc++ != '=')
+			goto bad_param;
+
+		/* retrieve parameter value */
+		ptr = kstrdup(eoc, GFP_ATOMIC);
+
+		/* queue event */
+		add_cid_event(cs, cid, rt->resp_code, ptr, 0);
+		break;
+
+	case RT_ZCAU:
+		/* check parameter separators */
+		if (*eoc++ != '=')
+			goto bad_param;
+		psep = strchr(eoc, ',');
+		if (!psep)
+			goto bad_param;
+		*psep++ = 0;
+
+		/* decode parameter values */
+		if (kstrtou8(eoc, 16, &type) || kstrtou8(psep, 16, &value)) {
+			*--psep = ',';
+			goto bad_param;
+		}
+		parameter = (type << 8) | value;
+
+		add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
+		break;
+
+	case RT_NUMBER:
+		/* check parameter separator */
+		if (*eoc++ != '=')
+			goto bad_param;
+
+		/* decode parameter value */
+		if (kstrtoint(eoc, 10, &parameter))
+			goto bad_param;
+
+		/* special case ZDLE: set flag before queueing event */
+		if (rt->resp_code == RSP_ZDLE)
+			cs->dle = parameter;
+
+		add_cid_event(cs, cid, rt->resp_code, NULL, parameter);
+		break;
+
+bad_param:
+		/* parameter unexpected, incomplete or malformed */
+		dev_warn(cs->dev, "bad parameter in response '%s'\n",
+			 cs->respdata);
+		add_cid_event(cs, cid, rt->resp_code, NULL, -1);
+		break;
+
+	default:
+		dev_err(cs->dev, "%s: internal error on '%s'\n",
+			__func__, cs->respdata);
+	}
 }
 EXPORT_SYMBOL_GPL(gigaset_handle_modem_response);

Signature · 1 line

1.9.2.459.g68773ac

David Miller

maintainer

2015-03-23 20:47 UTC #5 lore raw

In reply to Tilman Schmidt

From: Tilman Schmidt <[email protected]>
Date: Sat, 21 Mar 2015 20:15:32 +0100 (CET)

Quoted 4 lines

This series of patches restructures the Gigaset ISDN driver's modem response parser to improve code readability and conform better to the device's specification and actual behaviour. Could you please merge these through net-next?

Series applied to net-next, thanks.