All,
I have a problem with this bug report. There is nothing wrong with
implementation and the patch is fine, but it is useless. Why are we adding
these flags if they are not used? Is this a 'missed it last time' repaired
omission? If that is the case, this work should be delayed until such time
as the flags are needed. Or simply add documentation concerning how the
flags should be used but do not actually add the code until it is needed.
I think the bug report should be moved back to open and modified to actually
accomplish something.
We should not add code that doesn't do anything or isn't acted on --
normally it's a bomb waiting to explode. In this case, flags are innocuous
but the premise remains: bug reports should fix something. I don't see where
this fixes anything that is broken (see below).
I can make a case for adding code to process one of the flags: Shouldn't the
code that does the restore interrogate the flags to see if the image
contains a validity point and if so read it and report that information to
the user via the log (backup_history)? This, at least, accomplishes
something that is indeed missing in the code.
As for the endianess (sp?) flag... That smells of something much more
sinister. Why do we have the flag if we are not checking endianess on
restore? Is it possible to backup on a little endian and restore on a big
endian machine (or vice-versa)? If the answer is 'yes' then the flag is
useless and should not be added. If the answer is 'no' then perhaps this bug
report should be about that problem and therefore solve it.
I think also that part of the solution to this bug report should correctly
document the use of the flags.
Until these questions are answered and/or someone convinces me these changes
are needed even though they accomplish nothing, I cannot approve the patch.
Sorry. I hate when this happens to me so I do feel your stress and pain,
Jorgen. :)
Chuck
