From: Andrzej Zaborowski <balrog@zabor.org>

Date: Wed, 11 Jul 2007 16:03:16 +0200
Subject: [PATCH] Multiline commands support.  Miscellaneous bugs.

This adds support for commands like +CMGS and +CMGW that span mroe than one
line and the lines can't be sent to modem as separate commands because every
next line has to wait for a "> " prompt from modem before writing (otherwise
beginnings of the lines get eaten).

This patch also corrects a number of miscellaneous glitches that I have hit. I
can split them each out if needed, but they are all quite obvious. The
cms_error variable is introduced because there are CME and possibly other
errors with the same codes, which resulted in that when I was sending a message
and got error 42 ("congestion") on sending, openmoko-dialer asked for PIN
because CMS event 42 happens to be PIN inquiry. The change in libgsmd-tool
fixes the issue described by Philipp Zabel on the list, with usock locking up.


git-svn-id: http://svn.openmoko.org/trunk/src/target/gsm@2709 99fdad57-331a-0410-800a-d7fa5415bdb3

6 files changed, 97 insertions, 59 deletions

diff --git a/include/gsmd/gsmd.h b/include/gsmd/gsmd.h
index 0ce753b..4362999 100644
--- a/include/gsmd/gsmd.h
+++ b/include/gsmd/gsmd.h
@@ -27,6 +27,7 @@ struct gsmd_atcmd {
 	u_int32_t buflen;
 	u_int16_t id;
 	u_int8_t flags;
+	char *cur;
 	char buf[];
 };
 
@@ -36,6 +37,8 @@ enum llparse_state {
 	LLPARSE_STATE_IDLE_LF,		/* LF before response (V1) */
 	LLPARSE_STATE_RESULT,		/* within result payload */
 	LLPARSE_STATE_RESULT_CR,	/* CR after result */
+	LLPARSE_STATE_PROMPT,		/* within a "> " prompt */
+	LLPARSE_STATE_PROMPT_SPC,	/* a complete "> " prompt */
 	LLPARSE_STATE_ERROR,		/* something went wrong */
 					/* ... idle again */
 };
@@ -52,6 +55,7 @@ struct llparser {
 	unsigned int flags;
 	void *ctx;
 	int (*cb)(const char *buf, int len, void *ctx);
+	int (*prompt_cb)(void *ctx);
 	char *cur;
 	char buf[LLPARSE_BUF_SIZE];
 };
@@ -92,7 +96,8 @@ struct gsmd_user {
 
 extern int gsmdlog_init(const char *path);
 /* write a message to the daemons' logfile */
-void __gsmd_log(int level, const char *file, int line, const char *function, const char *message, ...);
+void __gsmd_log(int level, const char *file, int line, const char *function, const char *message, ...)
+	__attribute__ ((__format__ (__printf__, 5, 6)));
 /* macro for logging including filename and line number */
 #define gsmd_log(level, format, args ...) \
 	__gsmd_log(level, __FILE__, __LINE__, __FUNCTION__, format, ## args)
diff --git a/src/gsmd/atcmd.c b/src/gsmd/atcmd.c
index 5cb9298..506f7b8 100644
--- a/src/gsmd/atcmd.c
+++ b/src/gsmd/atcmd.c
@@ -82,9 +82,12 @@ static int llparse_byte(struct llparser *llp, char byte)
 
 	switch (llp->state) {
 	case LLPARSE_STATE_IDLE:
+	case LLPARSE_STATE_PROMPT_SPC:
 		if (llp->flags & LGSM_ATCMD_F_EXTENDED) {
 			if (byte == '\r')
 				llp->state = LLPARSE_STATE_IDLE_CR;
+			else if (byte == '>')
+				llp->state = LLPARSE_STATE_PROMPT;
 			else {
 #ifdef STRICT
 				llp->state = LLPARSE_STATE_ERROR;
@@ -108,6 +111,8 @@ static int llparse_byte(struct llparser *llp, char byte)
 		/* can we really go directly into result_cr ? */
 		if (byte == '\r')
 			llp->state = LLPARSE_STATE_RESULT_CR;
+		else if (byte == '>')
+			llp->state = LLPARSE_STATE_PROMPT;
 		else {
 			llp->state = LLPARSE_STATE_RESULT;
 			ret = llparse_append(llp, byte);
@@ -127,6 +132,16 @@ static int llparse_byte(struct llparser *llp, char byte)
 			memset(llp->buf, 0, LLPARSE_BUF_SIZE);
 		}
 		break;
+	case LLPARSE_STATE_PROMPT:
+		if (byte == ' ')
+			llp->state = LLPARSE_STATE_PROMPT_SPC;
+		else {
+			/* this was not a real "> " prompt */
+			llparse_append(llp, '>');
+			ret = llparse_append(llp, byte);
+			llp->state = LLPARSE_STATE_RESULT;
+		}
+		break;
 	case LLPARSE_STATE_ERROR:
 		break;
 	}
@@ -147,6 +162,10 @@ static int llparse_string(struct llparser *llp, char *buf, unsigned int len)
 			/* FIXME: what to do with return value ? */
 			llp->cb(llp->buf, llp->cur - llp->buf, llp->ctx);
 		}
+
+		/* if a full SMS-style prompt was received, poke the select */
+		if (llp->state == LLPARSE_STATE_PROMPT_SPC)
+			llp->prompt_cb(llp->ctx);
 	}
 
 	return 0;
@@ -176,8 +195,9 @@ static int ml_parse(const char *buf, int len, void *ctx)
 	struct gsmd *g = ctx;
 	struct gsmd_atcmd *cmd = NULL;
 	static char mlbuf[MLPARSE_BUF_SIZE];
-	int rc = 0, final = 0;
-	int mlbuf_len;
+	int rc = 0;
+	static int mlbuf_len;
+	int cme_error = 0;
 
 	DEBUGP("buf=`%s'(%d)\n", buf, len);
 
@@ -231,7 +251,7 @@ static int ml_parse(const char *buf, int len, void *ctx)
 			DEBUGP("error number %lu\n", err_nr);
 			if (cmd)
 				cmd->ret = err_nr;
-			final = 1;
+			cme_error = 1;
 			goto final_cb;
 		}
 		if (!strncmp(buf+1, "CMS ERROR", 9)) {
@@ -241,7 +261,6 @@ static int ml_parse(const char *buf, int len, void *ctx)
 			DEBUGP("error number %lu\n", err_nr);
 			if (cmd)
 				cmd->ret = err_nr;
-			final = 1;
 			goto final_cb;
 		}
 
@@ -273,7 +292,7 @@ static int ml_parse(const char *buf, int len, void *ctx)
 
 			/* it might be a multiline response, so if there's a previous
 			   response, send out mlbuf and start afresh with an empty buffer */
-			if (mlbuf[0] != 0) {
+			if (mlbuf_len) {
 				if (!cmd->cb) {
 					gsmd_log(GSMD_NOTICE, "command without cb!!!\n");
 				} else {
@@ -281,8 +300,8 @@ static int ml_parse(const char *buf, int len, void *ctx)
 					cmd->resp = mlbuf;
 					rc = cmd->cb(cmd, cmd->ctx, cmd->resp);
 					DEBUGP("Clearing mlbuf\n");
-					mlbuf[0] = 0;
 				}
+				mlbuf_len = 0;
 			}
 
 			/* the current buf will be appended to mlbuf below */
@@ -301,7 +320,6 @@ static int ml_parse(const char *buf, int len, void *ctx)
 			DEBUGP("unspecified error\n");
 			if (cmd)
 				cmd->ret = 4;
-			final = 1;
 			goto final_cb;
 		}
 
@@ -310,7 +328,6 @@ static int ml_parse(const char *buf, int len, void *ctx)
 			/* Part of Case 'C' */
 			if (cmd)
 				cmd->ret = 0;
-			final = 1;
 			goto final_cb;
 		}
 
@@ -319,14 +336,12 @@ static int ml_parse(const char *buf, int len, void *ctx)
 		if (!strncmp(buf, "NO CARRIER", 11) ||
 		    ((g->flags & GSMD_FLAG_V0) && buf[0] == '3')) {
 			/* Part of Case 'D' */
-			final = 1;
 			goto final_cb;
 		}
 
 		if (!strncmp(buf, "BUSY", 4) ||
 		    ((g->flags & GSMD_FLAG_V0) && buf[0] == '7')) {
 			/* Part of Case 'D' */
-			final = 1;
 			goto final_cb;
 		}
 	}
@@ -334,21 +349,13 @@ static int ml_parse(const char *buf, int len, void *ctx)
 	/* we reach here, if we are at an information response that needs to be
 	 * passed on */
 
-	if (mlbuf[0] == 0) {
-		DEBUGP("Filling mlbuf\n");
-		strncat(mlbuf, buf, sizeof(mlbuf)-1);
-	} else {
-		DEBUGP("Appending buf to mlbuf\n");
-		mlbuf_len = strlen(mlbuf);
-		if (mlbuf_len+1 < sizeof(mlbuf)) {
-			mlbuf[mlbuf_len] = '\n';
-			mlbuf[mlbuf_len+1] = '\0';
-			strncat(mlbuf, buf, sizeof(mlbuf)-mlbuf_len-2);
-		} else {
-			DEBUGP("response too big for mlbuf!!!\n");
-			return -EFBIG;
-		}
-	}
+	if (mlbuf_len)
+		mlbuf[mlbuf_len ++] = '\n';
+	DEBUGP("Appending buf to mlbuf\n");
+	if (len > sizeof(mlbuf) - mlbuf_len)
+		len = sizeof(mlbuf) - mlbuf_len;
+	memcpy(mlbuf + mlbuf_len, buf, len);
+	mlbuf_len += len;
 	return 0;
 
 final_cb:
@@ -357,7 +364,7 @@ final_cb:
 	if (!cmd)
 		return rc;
 
-	if (cmd && cmd->ret != 0)
+	if (cmd && cme_error)
 		generate_event_from_cme(g, cmd->ret);
 
 	if (!cmd->cb) {
@@ -365,13 +372,14 @@ final_cb:
 	} else {
 		DEBUGP("Calling final cmd->cb()\n");
 		/* send final result code if there is no information response in mlbuf */
-		if (mlbuf[0] == 0)
-			cmd->resp = buf;
-		else
+		if (mlbuf_len) {
 			cmd->resp = mlbuf;
+			mlbuf[mlbuf_len] = 0;
+		} else
+			cmd->resp = buf;
 		rc = cmd->cb(cmd, cmd->ctx, cmd->resp);
 		DEBUGP("Clearing mlbuf\n");
-		mlbuf[0] = 0;
+		mlbuf_len = 0;
 	}
 
 	/* remove from list of currently executing cmds */
@@ -384,7 +392,15 @@ final_cb:
 		g->gfd_uart.when |= GSMD_FD_WRITE;
 
 	return rc;
-}	
+}
+
+/* called when the modem asked for a new line of a multiline atcmd */
+static int atcmd_prompt(void *data)
+{
+	struct gsmd *g = data;
+
+	g->gfd_uart.when |= GSMD_FD_WRITE;
+}
 
 /* callback to be called if [virtual] UART has some data for us */
 static int atcmd_select_cb(int fd, unsigned int what, void *data)
@@ -392,6 +408,7 @@ static int atcmd_select_cb(int fd, unsigned int what, void *data)
 	int len, rc;
 	static char rxbuf[1024];
 	struct gsmd *g = data;
+	char *cr;
 
 	if (what & GSMD_FD_READ) {
 		memset(rxbuf, 0, sizeof(rxbuf));
@@ -415,8 +432,12 @@ static int atcmd_select_cb(int fd, unsigned int what, void *data)
 	if ((what & GSMD_FD_WRITE) && g->interpreter_ready) {
 		struct gsmd_atcmd *pos, *pos2;
 		llist_for_each_entry_safe(pos, pos2, &g->pending_atcmds, list) {
-			len = strlen(pos->buf);
-			rc = write(fd, pos->buf, strlen(pos->buf));
+			cr = strchr(pos->cur, '\n');
+			if (cr)
+				len = cr - pos->cur;
+			else
+				len = pos->buflen;
+			rc = write(fd, pos->cur, len);
 			if (rc == 0) {
 				gsmd_log(GSMD_ERROR, "write returns 0, aborting\n");
 				break;
@@ -425,27 +446,33 @@ static int atcmd_select_cb(int fd, unsigned int what, void *data)
 					fd, rc);
 				return rc;
 			}
-			if (rc < len) {
-				gsmd_log(GSMD_FATAL, "short write!!! FIXME!\n");
-				exit(3);
-			}
+			if (cr && rc == len)
+				rc ++;	/* Skip the \n */
+			pos->buflen -= rc;
+			pos->cur += rc;
 			write(fd, "\r", 1);
-			/* success: remove from global list of to-be-sent atcmds */
-			llist_del(&pos->list);
-			/* append to global list of executing atcmds */
-			llist_add_tail(&pos->list, &g->busy_atcmds);
+
+			if (!pos->buflen) {
+				/* success: remove from global list of
+				 * to-be-sent atcmds */
+				llist_del(&pos->list);
+				/* append to global list of executing atcmds */
+				llist_add_tail(&pos->list, &g->busy_atcmds);
 
 				/* we only send one cmd at the moment */
-				g->gfd_uart.when &= ~GSMD_FD_WRITE;
 				break;
+			} else {
+				/* The write was short or the atcmd has more
+				 * lines to send after a "> ".  */
+				if (rc < len)
+					return 0;
+				break;
+			}
 		}
-	}
 
-#if 0
-	if (llist_empty(&g->pending_atcmds))
+		/* Either pending_atcmds is empty or a command has to wait */
 		g->gfd_uart.when &= ~GSMD_FD_WRITE;
-#endif
-		
+	}
 
 	return 0;
 }
@@ -456,10 +483,10 @@ struct gsmd_atcmd *atcmd_fill(const char *cmd, int rlen,
 {
 	int buflen = strlen(cmd);
 	struct gsmd_atcmd *atcmd;
-	
+
 	if (rlen > buflen)
 		buflen = rlen;
-	
+
 	atcmd = talloc_size(__atcmd_ctx, sizeof(*atcmd)+ buflen);
 	if (!atcmd)
 		return NULL;
@@ -470,6 +497,7 @@ struct gsmd_atcmd *atcmd_fill(const char *cmd, int rlen,
 	atcmd->ret = -255;
 	atcmd->buflen = buflen;
 	atcmd->buf[buflen-1] = '\0';
+	atcmd->cur = atcmd->buf;
 	atcmd->cb = cb;
 	atcmd->resp = NULL;
 	strncpy(atcmd->buf, cmd, buflen-1);
@@ -482,8 +510,9 @@ int atcmd_submit(struct gsmd *g, struct gsmd_atcmd *cmd)
 {
 	DEBUGP("submitting command `%s'\n", cmd->buf);
 
+	if (llist_empty(&g->pending_atcmds))
+		g->gfd_uart.when |= GSMD_FD_WRITE;
 	llist_add_tail(&cmd->list, &g->pending_atcmds);
-	g->gfd_uart.when |= GSMD_FD_WRITE;
 
 	return 0;
 }
@@ -519,9 +548,9 @@ int atcmd_init(struct gsmd *g, int sockfd)
 	g->llp.cur = g->llp.buf;
 	g->llp.len = sizeof(g->llp.buf);
 	g->llp.cb = &ml_parse;
+	g->llp.prompt_cb = &atcmd_prompt;
 	g->llp.ctx = g;
 	g->llp.flags = LGSM_ATCMD_F_EXTENDED;
 
 	return gsmd_register_fd(&g->gfd_uart);
-}	
-
+}
diff --git a/src/gsmd/unsolicited.c b/src/gsmd/unsolicited.c
index b9bd97f..f9351d2 100644
--- a/src/gsmd/unsolicited.c
+++ b/src/gsmd/unsolicited.c
@@ -518,9 +518,11 @@ int generate_event_from_cme(struct gsmd *g, unsigned int cme_error)
 	case GSM0707_CME_PHONE_ADAPT_RESERVED:
 	case GSM0707_CME_SIM_NOT_INSERTED:
 		/* FIXME */
+		talloc_free(gu);
 		return 0;
 		break;
 	default:
+		talloc_free(gu);
 		return 0;
 		break;
 	}
diff --git a/src/gsmd/usock.c b/src/gsmd/usock.c
index 617c9f2..db073a4 100644
--- a/src/gsmd/usock.c
+++ b/src/gsmd/usock.c
@@ -75,7 +75,7 @@ static int usock_cmd_cb(struct gsmd_atcmd *cmd, void *ctx, char *resp)
 	ucmd->hdr.version = GSMD_PROTO_VERSION;
 	ucmd->hdr.msg_type = GSMD_MSG_PASSTHROUGH;
 	ucmd->hdr.msg_subtype = GSMD_PASSTHROUGH_RESP;
-	ucmd->hdr.len = strlen(resp)+1;
+	ucmd->hdr.len = rlen;
 	ucmd->hdr.id = cmd->id;
 	memcpy(ucmd->buf, resp, ucmd->hdr.len);
 
@@ -100,7 +100,7 @@ static int usock_rcv_passthrough(struct gsmd_user *gu, struct gsmd_msg_hdr *gph,
 
 static int usock_rcv_event(struct gsmd_user *gu, struct gsmd_msg_hdr *gph, int len)
 {
-	u_int32_t *evtmask = (u_int32_t *) ((char *)gph + sizeof(*gph), gph->id);
+	u_int32_t *evtmask = (u_int32_t *) ((char *)gph + sizeof(*gph));
 
 	if (len < sizeof(*gph) + sizeof(u_int32_t))
 		return -EINVAL;
diff --git a/src/gsmd/vendor_ti.c b/src/gsmd/vendor_ti.c
index 555afcb..c219355 100644
--- a/src/gsmd/vendor_ti.c
+++ b/src/gsmd/vendor_ti.c
@@ -277,7 +277,7 @@ static int ticalypso_detect(struct gsmd *g)
 
 static int ticalypso_initsettings(struct gsmd *g)
 {
-	int rc;
+	int rc = 0;
 	struct gsmd_atcmd *cmd;
 
 	/* use +CTZR: to report time zone changes */
diff --git a/src/util/atcmd.c b/src/util/atcmd.c
index 395f506..f3716d7 100644
--- a/src/util/atcmd.c
+++ b/src/util/atcmd.c
@@ -91,9 +91,11 @@ int atcmd_main(struct lgsm_handle *lgsmh)
 				continue;
 			}
 			printf("STR=`%s'\n", buf);
+
+			/* this is a synchronous call for a passthrough
+			 * command */
+			lgsm_passthrough(lgsmh, buf, rbuf, &rlen);
+			printf("RSTR=`%s'\n", rbuf);
 		}
-		/* this is a synchronous call for a passthrough command */
-		lgsm_passthrough(lgsmh, buf, rbuf, &rlen);
-		printf("RSTR=`%s'\n", rbuf);
 	}
 }