Chuck,
I agree that the code in stream library is not good and should be fixed.
Chuck Bell wrote:
> Hi Svoj,
>
> Thanks for the review.
>
> Rafal: please comment on the discussion below.
>
>>> diff -Nrup a/sql/backup/stream_v1.c b/sql/backup/stream_v1.c
>>> --- a/sql/backup/stream_v1.c 2008-04-17 06:42:27 -04:00
>>> +++ b/sql/backup/stream_v1.c 2008-04-25 11:31:25 -04:00
>>> @@ -424,6 +424,7 @@ int bstream_rd_header(backup_stream *s,
>>> - 1 = snapshot created by built-in blocking driver (BI_DEFAULT),
>>> - 2 = snapshot created using created by built-in driver
>> using consistent
>>> read transaction (BI_CS).
>>> + - 3 = snapshot created by built-in no data driver (BI_NODATA),
>>>
>>> Format of [backup engine info] depends on snapshot type.
>> It is empty for the
>>> default and CS snapshots. For native snapshots it has format @@
>>> -446,7 +447,8 @@ int bstream_wr_snapshot_info(backup_stre
>>> case BI_NATIVE: ret= bstream_wr_byte(s,0); break;
>>> case BI_DEFAULT: ret= bstream_wr_byte(s,1); break;
>>> case BI_CS: ret= bstream_wr_byte(s,2); break;
>>> - default: ret= bstream_wr_byte(s,3); break;
>>> + case BI_NODATA: ret= bstream_wr_byte(s,3); break;
>>> + default: ret= bstream_wr_byte(s,4); break;
>>> }
>>>
>> How can we enter the "default" case here? I think it is
>> impossible, so I'd suggest to either report an error or
>> DBUG_ASSERT(0).
>
Yes, DBUG_ASSERT(0) would be better - the default case should never happen.
> You have a point. I am not sure. I recommend we open a new bug on this since
> this is part of the original stream work and not something this worklog is
> addressing. Shall I open the bug report?
>
>>
>>> @@ -495,6 +497,7 @@ int bstream_rd_image_info(backup_stream
>>> case 0: type= BI_NATIVE; break;
>>> case 1: type= BI_DEFAULT; break;
>>> case 2: type= BI_CS; break;
>>> + case 3: type= BI_NODATA; break;
>>> default: return BSTREAM_ERROR;
>>> }
>> Because at least here it wouldn't be accepted anyway. So why
>> do we need to create wittingly broken image file?
>
Agree, not a good idea to write unknown snapshot type id to the image. But above
code is ok - it will err if we have something unexpected in the stream.
> Again, I am not sure. I can add this to the bug report too.
>
Yes, would be good to report a bug on this.
Rafal
