#At file:///ext/mysql/bzr/backup/bug37522/
2691 Rafal Somla 2008-09-04
BUG#37522 (Backup: stream_v1_transport: Error condition should be checked for as_* calls)
This patch adds checks for errors in stream_v1_transport.c where the low level I/O
functions are called.
modified:
sql/backup/stream_v1.h
sql/backup/stream_v1_transport.c
per-file messages:
sql/backup/stream_v1.h
Add specifications for the low-level I/O functions implementing abstract stream
interface.
sql/backup/stream_v1_transport.c
- Make sure error is reported and stream state set to ERROR whenever the low-level I/O functions report error.
- Turn append_to_buffer() into void function as it can never error.
=== modified file 'sql/backup/stream_v1.h'
--- a/sql/backup/stream_v1.h 2008-05-14 00:35:24 +0000
+++ b/sql/backup/stream_v1.h 2008-09-04 09:01:30 +0000
@@ -378,6 +378,16 @@ int bstream_next_chunk(backup_stream*);
the bytes being written to/read from a backup stream. This is done by
storing pointers to functions performing basic I/O operations inside
this structure.
+
+ Low-level @c write(), @c read() and @c forward() functions should return
+ BSTREAM_OK on success and BSTREAM_ERROR on error. Additionally, @c read()
+ should return BSTREAM_EOS if there is no more data in the input stream.
+
+ Functions @c write() and @c read() should behave like functions
+ @c bstream_write_part() and @c bstream_read_part(), respectively. The latter
+ are documented in stream_v1_transport.c. The only difference is that low-level
+ @c read() doesn't deal with chunk boundaries and never returns BSTREAM_EOC.
+ However, it should detect end of stream and return BSTREAM_EOS in that case.
*/
struct st_abstract_stream
{
=== modified file 'sql/backup/stream_v1_transport.c'
--- a/sql/backup/stream_v1_transport.c 2008-07-19 03:03:39 +0000
+++ b/sql/backup/stream_v1_transport.c 2008-09-04 09:01:30 +0000
@@ -386,7 +386,14 @@ int write_buffer(backup_stream *s)
int ret= BSTREAM_OK;
if (s->buf.pos > s->buf.begin)
+ {
ret= as_write_all(&s->stream,*(bstream_blob*)&s->buf,s->mem);
+ if (ret != BSTREAM_OK)
+ {
+ s->state= ERROR;
+ return BSTREAM_ERROR;
+ }
+ }
/*
Now buffer should be empty. If whole block has been written, reset buffer
@@ -398,7 +405,7 @@ int write_buffer(backup_stream *s)
else
s->buf.begin= s->buf.header= s->buf.pos; /* now invariant should hold */
- return ret;
+ return BSTREAM_OK;
}
/**
@@ -408,18 +415,16 @@ int write_buffer(backup_stream *s)
is left for fragment`s header.
*/
static
-int append_to_buffer(backup_stream *s, blob *b)
+void append_to_buffer(backup_stream *s, blob *b)
{
if (b->begin >= b->end) /* no data to append */
- return BSTREAM_OK;
+ return;
if(s->buf.pos == s->buf.header) /* current fragment empty */
s->buf.pos++;
while ((s->buf.pos < s->buf.end) && (b->begin < b->end))
*(s->buf.pos++)= *(b->begin++);
-
- return BSTREAM_OK;
}
/**
@@ -658,6 +663,12 @@ int load_buffer(backup_stream *s)
ret= as_read(&s->stream,&data,s->mem);
+ if (ret == BSTREAM_ERROR)
+ {
+ s->state= ERROR;
+ return BSTREAM_ERROR;
+ }
+
s->buf.pos= data.begin;
/*
@@ -751,6 +762,7 @@ int load_buffer(backup_stream *s)
@retval BSTREAM_OK next fragment has been entered
@retval BSTREAM_EOC the entered fragment is the last fragment of a chunk
@retval BSTREAM_EOS there are no more fragments in the stream
+ @retval BSTREAM_ERROR error when reading from underlying stream
*/
static
int load_next_fragment(backup_stream *s)
@@ -766,7 +778,7 @@ int load_next_fragment(backup_stream *s)
s->state= NORMAL; /* default, unless changed below */
- ret= read_fragment_header(&s->buf.header);
+ ret= read_fragment_header(&s->buf.header); // Never errors.
/*
If buf.header was not moved, it means that the fragment extends to
@@ -1052,7 +1064,10 @@ int bstream_write_part(backup_stream *s,
ret= as_write_all(&s->stream,*data,buf);
if (ret != BSTREAM_OK)
+ {
+ s->state= ERROR;
return ret;
+ }
data->begin= data->end;
data->end= saved_end;
@@ -1088,6 +1103,11 @@ int bstream_write_part(backup_stream *s,
data->end= data->begin + (fragment.begin - s->buf.pos);
ret= as_write_all(&s->stream,*data,*data);
+ if (ret != BSTREAM_OK)
+ {
+ s->state= ERROR;
+ return ret;
+ }
data->begin= data->end;
data->end= saved_end;
@@ -1185,6 +1205,8 @@ int bstream_end_chunk(backup_stream *s)
else
{
ret= close_current_fragment(s);
+ if (ret != BSTREAM_OK)
+ return BSTREAM_ERROR;
/*
If the last fragment in the buffer is a small fragment, we can mark it as
@@ -1423,7 +1445,6 @@ int bstream_read_part(backup_stream *s,
if ((s->buf.begin == s->buf.end) || (s->buf.begin == s->buf.header))
{
ret= load_buffer(s);
-
if (ret != BSTREAM_OK)
return ret;
}
@@ -1482,11 +1503,17 @@ int bstream_read_part(backup_stream *s,
saved= *data;
data->end= data->begin + howmuch;
- if (as_read(&s->stream, data, buf) == BSTREAM_EOS)
+ ret= as_read(&s->stream, data, buf);
+
+ if (ret == BSTREAM_ERROR)
{
- s->state= EOS;
+ s->state= ERROR;
+ return BSTREAM_ERROR;
}
+ if (ret == BSTREAM_EOS)
+ s->state= EOS;
+
s->buf.begin += data->begin - saved.begin;
s->buf.pos= s->buf.begin;
@@ -1592,7 +1619,14 @@ int bstream_next_chunk(backup_stream *s)
else
{
howmuch= s->buf.header - s->buf.pos;
- as_forward(&s->stream, &howmuch);
+ ret= as_forward(&s->stream, &howmuch);
+
+ if (ret != BSTREAM_OK)
+ {
+ s->state= ERROR;
+ return BSTREAM_ERROR;
+ }
+
s->buf.begin= s->buf.pos= s->buf.header;
}
}
@@ -1621,6 +1655,9 @@ int bstream_next_chunk(backup_stream *s)
ret= load_next_fragment(s);
+ if (ret == BSTREAM_ERROR) // To avoid invariant check below.
+ return ret;
+
/* if we hit EOC marker here, we treat it as empty chunk */
if (ret == BSTREAM_EOC)
ret= BSTREAM_OK;
@@ -1638,5 +1675,13 @@ int bstream_next_chunk(backup_stream *s)
*/
int bstream_skip(backup_stream *s, unsigned long int howmuch)
{
- return as_forward(&s->stream, &howmuch);
+ int ret= as_forward(&s->stream, &howmuch);
+
+ if (ret != BSTREAM_OK)
+ {
+ s->state= ERROR;
+ return BSTREAM_ERROR;
+ }
+
+ return BSTREAM_OK;
}
