Re: [PATCH] FCI work for cabinet.dll [cabinet-fci-patch-03c.diff]

[Mike McCormack]

Hi Gerold

Gerold J. Wucherpfennig wrote:
> And now take this...

Gulp.
> +    p_fci_internal->perf->erfOper = FCIERR_NONE;
> +    p_fci_internal->perf->erfType = ERROR_INVALID_DATA;
> +    p_fci_internal->perf->fError = TRUE;
> +    SetLastError(ERROR_INVALID_DATA);

Your code seem to be doing this over and over again.  Is there any 
reason not to make a macro so you can do something like:

FCI_SET_ERROR(FCIERR_NONE, ERROR_INVALID_DATA );

maybe something like this:

#define FCI_SET_ERROR( oper, type )    \
  p_fci_internal->perf->erfOper = oper; \
  p_fci_internal->perf->erfType = type; \
  p_fci_internal->perf->fError = TRUE;  \
  SetLastError(type);

Also it would be nice to split this into two patches, one changing all 
the error codes as per above, and one with all the other minor changes 
you made.  A 75k patch will take a long time for Alexandre to digest...

Mike

[PATCH] FCI work for cabinet.dll [cabinet-fci-patch-03c.diff]

[Gerold J. Wucherpfennig]

On Sunday 03 July 2005 03:07, Mike McCormack wrote:
> Hi Gerold
>
> Gerold J. Wucherpfennig wrote:
> > And now take this...
>
> Gulp.
> > +    p_fci_internal->perf->erfOper = FCIERR_NONE;
> > +    p_fci_internal->perf->erfType = ERROR_INVALID_DATA;
> > +    p_fci_internal->perf->fError = TRUE;
> > +    SetLastError(ERROR_INVALID_DATA);
>
> Your code seem to be doing this over and over again.  Is there any
> reason not to make a macro so you can do something like:
>
> FCI_SET_ERROR(FCIERR_NONE, ERROR_INVALID_DATA );

OK I will review it and make something like that.
> maybe something like this:
>
> #define FCI_SET_ERROR( oper, type )    \
>   p_fci_internal->perf->erfOper = oper; \
>   p_fci_internal->perf->erfType = type; \
>   p_fci_internal->perf->fError = TRUE;  \
>   SetLastError(type);
>
> Also it would be nice to split this into two patches, one changing all
> the error codes as per above, and one with all the other minor changes
> you made.  A 75k patch will take a long time for Alexandre to digest...

OK now I'm busy with setting up svk and when that's done I'll examine how
svk can make patch management easier.

It was no fun creating and updating patches manually.
Alexandre, please remit the inconvieniences I made. (I don't know if that's
proper English)


Gerold