Rafal,
This patch is ok to push under the following conditions:
1) you merge the latest clone of mysql-6.0-backup (I know -- I'm nagging)
2) I think you need to remove the dead diffs. There are a lot of spacing
changes and things like:
-*/
+*/
...that occur mostly in stream_v1.h.
3) please check your condition statements and use ()'s more. For example, (a
&& b < c) is perfectly fine for the compiler but humans (especially this
one) have trouble reading things like that -- extra proof when there are 4
or more operands or if someone (like me) accidentally reorders them! Having
()'s around every comparison and op/operand pairs will help prevent errors.
I think this should be fixed to make debugging and reading of the code (for
comprehension) easier.
There are a few things I think should be changed but are not crucial to our
deadline:
1) Why did you change backup_stream->state to backup_stream->mode? State
seemed to be much more appropriate.
2) I think we need (collectively) to change our practices with enums. For
enums that are public, I think we need to either declare a scope with them
or make them unique enough that someone else can't accidentally reuse them.
For example, use scope::MY_ENUM or change things like TABLE_ITEM to
BSTREAM_TABLE_ITEM. This isn't just for your patch, mine need this too! ;)
Chuck
> -----Original Message-----
> From: rsomla@stripped [mailto:rsomla@stripped]
> Sent: Tuesday, November 27, 2007 5:04 AM
> To: commits@stripped
> Subject: bk commit into 6.0 tree (rafal:1.2669) WL#4063
>
> Below is the list of changes that have just been committed
> into a local
> 6.0 repository of rafal. When rafal does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-11-27 11:03:52+01:00, rafal@quant.(none) +4 -0
> WL#4063 (backup stream library):
>
> This patch introduces changes and bug fixes necessary for
> integrating the backup stream library
> with the backup server code.
>
> sql/backup/stream_v1.c@stripped, 2007-11-27 11:03:45+01:00,
> rafal@quant.(none) +115 -96
> - changed the way table position is stored in the
> catalogue: as snapshot number and position within snapshot's
> table list;
> - added checks for return values from the bcat_* service
> functions;
> - since item types are stored in 2 bytes, an item list
> separator consists of two 0x00 bytes now - updated docs
> and code;
> - when metadata item is read from a stream, the code
> doesn't try to locate it in the catalogue - the upper
> layer can do it on its own; this removes a need for
> bcat_get_*() service functions;
>
>
