I am satisfied. This patch (pre-merge) can be pushed.
> -----Original Message-----
> From: Rafal Somla [mailto:rsomla@stripped]
> Sent: Thursday, November 29, 2007 3:10 PM
> To: Chuck Bell
> Cc: commits@stripped
> Subject: Re: bk commit into 6.0 tree (rafal:1.2669) WL#4063
>
> Before merge comments for the second patch (changes to backup
> stream library).
>
> Chuck Bell wrote:
> > 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)
>
> Will come as a separate patch.
>
> > 2) I think you need to remove the dead diffs. There are a lot of
> > spacing changes and things like:
> >
> > -*/
> > +*/
> >
>
> These changes remove spaces at end of lines (which should not
> be there). I have a script which does it automatically before
> a commit - if spaces get there somehow, such diffs result.
> Removing them would be extremely tedious and time consuming
> since I'd have to discover in what places there were spaces
> and restore exact the same amount. I'd be grateful if we can
> leave it for now (I know it is annoying when reviewing).
>
> After long fight with bk I did this - moved all space changes
> to a new rename patch. After that, the two other patches
> should be free from such diffs I hope.
>
> I should find a better solution for "spurious spaces" problem
> but I never have time to work on this.
>
> > ...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.
> >
>
> I added parentheses
>
> > There are a few things I think should be changed but are
> not crucial
> > to our
> > deadline:
> >
>
> OK, let discuss them after the push then.
>
> I also added definition of bzero() using memset in hope of
> avoiding windows compilation problems.
>
>
> > 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;
> >>
> >>
> >
