Commit 65fd8474 authored by Colin Watson's avatar Colin Watson
Browse files

Fix EOF detection in get_line

* lib/pipeline.c (get_line): A short read isn't a reliable way to detect
end-of-file.  Instead, keep track of the previous buffer length returned
by get_block; if we get the same length twice in a row then that
indicates EOF.
* tests/reading_long_line.c: Rename to ...
* tests/read.c: ... this.  Update build system and test names to match.
(slow_line_helper, test_read_readline_slow): New test.
parent 2dd24921
......@@ -36,5 +36,5 @@ tests/basic
tests/exec
tests/inspect
tests/pump
tests/reading_long_line
tests/read
tests/redirect
......@@ -2214,6 +2214,7 @@ static const char *get_line (pipeline *p, size_t *outlen)
const size_t block = 4096;
const char *buffer = NULL, *end = NULL;
int i;
size_t previous_len = 0;
if (p->line_cache) {
free (p->line_cache);
......@@ -2224,18 +2225,21 @@ static const char *get_line (pipeline *p, size_t *outlen)
*outlen = 0;
for (i = 0; ; ++i) {
size_t plen = block * (i + 1);
size_t len = block * (i + 1);
buffer = get_block (p, &plen, 1);
if (!buffer || plen == 0)
buffer = get_block (p, &len, 1);
if (!buffer || len == 0)
return NULL;
end = memchr (buffer + block * i, '\n', plen - block * i);
if (!end && plen < block * (i + 1))
if (len == previous_len)
/* end of file, no newline found */
end = buffer + plen - 1;
end = buffer + len - 1;
else
end = memchr (buffer + previous_len, '\n',
len - previous_len);
if (end)
break;
previous_len = len;
}
if (end) {
......
......@@ -27,8 +27,8 @@ TESTS = \
exec \
inspect \
pump \
redirect \
reading_long_line
read \
redirect
check_PROGRAMS = $(TESTS)
LIBS = ../gnulib/lib/libgnu.la $(LTLIBOBJS) $(top_builddir)/lib/libpipeline.la
......@@ -57,8 +57,8 @@ inspect_LDADD = $(LIBS) @CHECK_LIBS@
pump_SOURCES = pump.c common.c common.h
pump_LDADD = $(LIBS) @CHECK_LIBS@
read_SOURCES = read.c common.c common.h
read_LDADD = $(LIBS) @CHECK_LIBS@
redirect_SOURCES = redirect.c common.c common.h
redirect_LDADD = $(LIBS) @CHECK_LIBS@
reading_long_line_SOURCES = reading_long_line.c common.c common.h
reading_long_line_LDADD = $(LIBS) @CHECK_LIBS@
......@@ -88,7 +88,7 @@ POST_UNINSTALL = :
build_triplet = @build@
host_triplet = @host@
TESTS = basic$(EXEEXT) argstr$(EXEEXT) exec$(EXEEXT) inspect$(EXEEXT) \
pump$(EXEEXT) redirect$(EXEEXT) reading_long_line$(EXEEXT)
pump$(EXEEXT) read$(EXEEXT) redirect$(EXEEXT)
check_PROGRAMS = $(am__EXEEXT_1)
subdir = tests
ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
......@@ -197,8 +197,7 @@ CONFIG_HEADER = $(top_builddir)/config.h
CONFIG_CLEAN_FILES =
CONFIG_CLEAN_VPATH_FILES =
am__EXEEXT_1 = basic$(EXEEXT) argstr$(EXEEXT) exec$(EXEEXT) \
inspect$(EXEEXT) pump$(EXEEXT) redirect$(EXEEXT) \
reading_long_line$(EXEEXT)
inspect$(EXEEXT) pump$(EXEEXT) read$(EXEEXT) redirect$(EXEEXT)
am_argstr_OBJECTS = argstr.$(OBJEXT) common.$(OBJEXT)
argstr_OBJECTS = $(am_argstr_OBJECTS)
argstr_DEPENDENCIES = $(LIBS)
......@@ -218,10 +217,9 @@ inspect_DEPENDENCIES = $(LIBS)
am_pump_OBJECTS = pump.$(OBJEXT) common.$(OBJEXT)
pump_OBJECTS = $(am_pump_OBJECTS)
pump_DEPENDENCIES = $(LIBS)
am_reading_long_line_OBJECTS = reading_long_line.$(OBJEXT) \
common.$(OBJEXT)
reading_long_line_OBJECTS = $(am_reading_long_line_OBJECTS)
reading_long_line_DEPENDENCIES = $(LIBS)
am_read_OBJECTS = read.$(OBJEXT) common.$(OBJEXT)
read_OBJECTS = $(am_read_OBJECTS)
read_DEPENDENCIES = $(LIBS)
am_redirect_OBJECTS = redirect.$(OBJEXT) common.$(OBJEXT)
redirect_OBJECTS = $(am_redirect_OBJECTS)
redirect_DEPENDENCIES = $(LIBS)
......@@ -260,11 +258,11 @@ am__v_CCLD_ = $(am__v_CCLD_@AM_DEFAULT_V@)
am__v_CCLD_0 = @echo " CCLD " $@;
am__v_CCLD_1 =
SOURCES = $(argstr_SOURCES) $(basic_SOURCES) $(exec_SOURCES) \
$(inspect_SOURCES) $(pump_SOURCES) \
$(reading_long_line_SOURCES) $(redirect_SOURCES)
$(inspect_SOURCES) $(pump_SOURCES) $(read_SOURCES) \
$(redirect_SOURCES)
DIST_SOURCES = $(argstr_SOURCES) $(basic_SOURCES) $(exec_SOURCES) \
$(inspect_SOURCES) $(pump_SOURCES) \
$(reading_long_line_SOURCES) $(redirect_SOURCES)
$(inspect_SOURCES) $(pump_SOURCES) $(read_SOURCES) \
$(redirect_SOURCES)
am__can_run_installinfo = \
case $$AM_UPDATE_INFO_DIR in \
n|no|NO) false;; \
......@@ -1290,10 +1288,10 @@ inspect_SOURCES = inspect.c common.c common.h
inspect_LDADD = $(LIBS) @CHECK_LIBS@
pump_SOURCES = pump.c common.c common.h
pump_LDADD = $(LIBS) @CHECK_LIBS@
read_SOURCES = read.c common.c common.h
read_LDADD = $(LIBS) @CHECK_LIBS@
redirect_SOURCES = redirect.c common.c common.h
redirect_LDADD = $(LIBS) @CHECK_LIBS@
reading_long_line_SOURCES = reading_long_line.c common.c common.h
reading_long_line_LDADD = $(LIBS) @CHECK_LIBS@
all: all-am
.SUFFIXES:
......@@ -1357,9 +1355,9 @@ pump$(EXEEXT): $(pump_OBJECTS) $(pump_DEPENDENCIES) $(EXTRA_pump_DEPENDENCIES)
@rm -f pump$(EXEEXT)
$(AM_V_CCLD)$(LINK) $(pump_OBJECTS) $(pump_LDADD) $(LIBS)
reading_long_line$(EXEEXT): $(reading_long_line_OBJECTS) $(reading_long_line_DEPENDENCIES) $(EXTRA_reading_long_line_DEPENDENCIES)
@rm -f reading_long_line$(EXEEXT)
$(AM_V_CCLD)$(LINK) $(reading_long_line_OBJECTS) $(reading_long_line_LDADD) $(LIBS)
read$(EXEEXT): $(read_OBJECTS) $(read_DEPENDENCIES) $(EXTRA_read_DEPENDENCIES)
@rm -f read$(EXEEXT)
$(AM_V_CCLD)$(LINK) $(read_OBJECTS) $(read_LDADD) $(LIBS)
redirect$(EXEEXT): $(redirect_OBJECTS) $(redirect_DEPENDENCIES) $(EXTRA_redirect_DEPENDENCIES)
@rm -f redirect$(EXEEXT)
......@@ -1377,7 +1375,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/exec.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/inspect.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/pump.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/reading_long_line.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/read.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/redirect.Po@am__quote@
.c.o:
......@@ -1635,16 +1633,16 @@ pump.log: pump$(EXEEXT)
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
redirect.log: redirect$(EXEEXT)
@p='redirect$(EXEEXT)'; \
b='redirect'; \
read.log: read$(EXEEXT)
@p='read$(EXEEXT)'; \
b='read'; \
$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
"$$tst" $(AM_TESTS_FD_REDIRECT)
reading_long_line.log: reading_long_line$(EXEEXT)
@p='reading_long_line$(EXEEXT)'; \
b='reading_long_line'; \
redirect.log: redirect$(EXEEXT)
@p='redirect$(EXEEXT)'; \
b='redirect'; \
$(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \
--log-file $$b.log --trs-file $$b.trs \
$(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \
......
......@@ -19,16 +19,16 @@
* USA.
*/
/*
* Unit test for bug: https://bugzilla.redhat.com/show_bug.cgi?id=876108
*/
#ifdef HAVE_CONFIG_H
# include "config.h"
#endif
#include <string.h>
#include <sys/select.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include "xalloc.h"
#include "xvasprintf.h"
#include "common.h"
......@@ -38,7 +38,8 @@ const char *program_name = "reading_long_line";
/* Must be 8194 or bigger */
#define RANDOM_STR_LEN 9000
START_TEST (test_reading_longline)
/* Unit test for bug: https://bugzilla.redhat.com/show_bug.cgi?id=876108 */
START_TEST (test_read_long_line)
{
/* Generate long random string */
static const char *alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
......@@ -111,14 +112,49 @@ START_TEST (test_reading_longline)
}
END_TEST
Suite *reading_long_line_suite (void)
/* Write the first character of a string quickly, followed by the rest of it
* a little later.
*/
static void slow_line_helper (void *data)
{
const char *s = data;
struct timeval timeout;
setbuf (stdout, NULL);
putchar (s[0]);
timeout.tv_sec = 0;
timeout.tv_usec = 100000;
select (0, NULL, NULL, NULL, &timeout);
fputs (s + 1, stdout);
}
START_TEST (test_read_readline_slow)
{
pipeline *p;
pipecmd *cmd;
const char *line;
p = pipeline_new ();
cmd = pipecmd_new_function ("slow_line_helper", slow_line_helper,
free, xstrdup ("a line\nsome more"));
pipeline_command (p, cmd);
pipeline_want_out (p, -1);
pipeline_start (p);
line = pipeline_readline (p);
fail_unless (!strcmp (line, "a line\n"));
pipeline_free (p);
}
END_TEST
Suite *read_suite (void)
{
Suite *s = suite_create ("Reading long line");
Suite *s = suite_create ("Read");
TEST_CASE_WITH_FIXTURE (s, reading, longline,
temp_dir_setup, temp_dir_teardown);
TEST_CASE_WITH_FIXTURE (s, read, long_line,
temp_dir_setup, temp_dir_teardown);
TEST_CASE (s, read, readline_slow);
return s;
}
MAIN (reading_long_line)
MAIN (read)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment