Document the parser interface and remove some excessive readdir paranoia.

5 files changed, 62 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index 5af81f57..192952b2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2012-07-26         Andrew J. Schorr     <aschorr@telemetry-investments.com>
 
+	* gawkapi.h (IOBUF_PUBLIC): Document the get_record and close_func
+	API.
+	(awk_input_parser_t) Change can_take_file argument to const, and
+	document the API.
+	* io.c (get_a_record): Document that the caller initializes *errcode
+	to 0, and remote the test for non-NULL errcode.
+
+2012-07-26         Andrew J. Schorr     <aschorr@telemetry-investments.com>
+
 	* gawkapi.c (api_sym_update_scalar): Fix some minor bugs.  Was
 	not updating AWK_NUMBER when valref != 1.  And strings were not
 	freeing MPFR values.
diff --git a/extension/ChangeLog b/extension/ChangeLog
index 1836b2cc..e2e0b635 100644
--- a/extension/ChangeLog
+++ b/extension/ChangeLog
@@ -1,3 +1,9 @@
+2012-07-26         Andrew J. Schorr     <aschorr@telemetry-investments.com>
+
+	* readdir.c (dir_get_record): No need to set *errcode to 0.
+	(dir_take_control_of): Remove some paranoia -- no need to test for
+	NULL iobuf, and no need to check dir_can_take_file again.
+
 2012-07-25         Arnold D. Robbins     <arnold@skeeve.com>
 
 	* readdir.c: New file.
diff --git a/extension/readdir.c b/extension/readdir.c
index a39e0c3a..c838ea72 100644
--- a/extension/readdir.c
+++ b/extension/readdir.c
@@ -106,8 +106,10 @@ dir_get_record(char **out, struct iobuf_public *iobuf, int *errcode)
 	if (out == NULL || iobuf == NULL || iobuf->opaque == NULL)
 		return EOF;
 
-	if (errcode != NULL)
-		*errcode = 0;
+	/*
+	 * The caller sets *errcode to 0, so we should set it only if an
+	 * error occurs.
+	 */
 
 	/* FIXME: Need stuff for setting RT */
 	dp = (DIR *) iobuf->opaque;
@@ -152,32 +154,23 @@ dir_can_take_file(IOBUF_PUBLIC *iobuf)
 	return (fd >= 0 && fstat(fd, & sbuf) >= 0 && S_ISDIR(sbuf.st_mode));
 }
 
-/* dir_take_control_of --- set up input parser */
+/* dir_take_control_of --- set up input parser.  We can assume that dir_can_take_file just returned true, and no state has changed since then. */
 
 static int
 dir_take_control_of(IOBUF_PUBLIC *iobuf)
 {
 	struct stat sbuf;
-	int fd;
-	DIR *dp = NULL;
+	DIR *dp;
 
-	if (iobuf == NULL)
+	dp = fdopendir(iobuf->fd);
+	if (dp == NULL)
 		return 0;
 
-	fd = iobuf->fd;
-	if (dir_can_take_file(iobuf)) {
-		dp = fdopendir(fd);
-		if (dp == NULL)
-			return 0;
-
-		iobuf->opaque = dp;
-		iobuf->get_record = dir_get_record;
-		iobuf->close_func = dir_close;
-
-		return 1;
-	}
+	iobuf->opaque = dp;
+	iobuf->get_record = dir_get_record;
+	iobuf->close_func = dir_close;
 
-	return 0;
+	return 1;
 }
 
 static awk_input_parser_t readdir_parser = {
diff --git a/gawkapi.h b/gawkapi.h
index b8422db5..bc7001c7 100644
--- a/gawkapi.h
+++ b/gawkapi.h
@@ -81,13 +81,45 @@ typedef struct iobuf_public {
 	const char *name;	/* filename */
 	int fd;			/* file descriptor */
 	void *opaque;           /* private data for open hooks */
+	/*
+	 * The get_record function is called to read the next record of data.
+	 * It should return the length of the input record (or EOF), and
+	 * it should set *out to point to the contents of $0.  Note that
+	 * gawk will make a copy of the record in *out, so the parser is
+	 * responsible for managing its own memory buffer.  If an error
+	 * occurs, the function should return EOF and set *errcode
+	 * to a non-zero value.  In that case, if *errcode does not equal
+	 * -1, gawk will automatically update the ERRNO variable based on
+	 * the value of *errcode (e.g. setting *errcode = errno should do
+	 * the right thing).  It is guaranteed that errcode is a valid
+	 * pointer, so there is no need to test for a NULL value.  The
+	 * caller sets *errcode to 0, so there is no need to set it unless
+	 * an error occurs.
+	 */
 	int (*get_record)(char **out, struct iobuf_public *, int *errcode);
+	/*
+	 * The close_func is called to allow the parser to free private data.
+	 * Gawk itself will close the fd unless close_func sets it to -1.
+	 */
 	void (*close_func)(struct iobuf_public *);
+							
 } IOBUF_PUBLIC;
 
 typedef struct input_parser {
 	const char *name;	/* name of parser */
-	int (*can_take_file)(IOBUF_PUBLIC *iobuf);
+	/*
+	 * The can_take_file function should return non-zero if the parser
+	 * would like to parse this file.  It should not change any gawk
+	 * state!
+	 */
+	int (*can_take_file)(const IOBUF_PUBLIC *iobuf);
+	/*
+	 * If this parser is selected, then take_control_of will be called.
+	 * It can assume that a previous call to can_take_file was successful,
+	 * and no gawk state has changed since that call.  It should populate
+	 * the IOBUF_PUBLIC get_record, close_func, and opaque values as needed.
+	 * It should return non-zero if successful.
+	 */
 	int (*take_control_of)(IOBUF_PUBLIC *iobuf);
 	struct input_parser *awk_const next;	/* for use by gawk */
 } awk_input_parser_t;
diff --git a/io.c b/io.c
index 6cfc3efe..05e87907 100644
--- a/io.c
+++ b/io.c
@@ -3036,7 +3036,7 @@ find_longest_terminator:
 	return REC_OK;
 }
 
-/* get_a_record --- read a record from IOP into out, return length of EOF, set RT */
+/* get_a_record --- read a record from IOP into out, return length of EOF, set RT.  Note that errcode is never NULL, and the caller initializes *errcode to 0. */
 
 static int
 get_a_record(char **out,        /* pointer to pointer to data */
@@ -3071,8 +3071,7 @@ get_a_record(char **out,        /* pointer to pointer to data */
 			return EOF;
 		} else if (iop->count == -1) {
 			iop->flag |= IOP_AT_EOF; 
-			if (errcode != NULL)
-				*errcode = errno;
+			*errcode = errno;
 			return EOF;
 		} else {
 			iop->dataend = iop->buf + iop->count;