Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane wrote:
>> Hmm, need more caffeine I guess. I was thinking dword == long. But in
>> any case, I'd feel a lot more comfortable if the patch ifdef'd the
>> declaration of exit_status to match, rather than forcing a cast of the
>> pointer value.
> Hmm. I looked at that, but that kind of just moves things around.
> If i change that variable to be DWORD, it still stuffs it into
> statuses[i] three lines further down,
Sure, but that's a plain old assignment that can cope with the two
variables being of different widths, so long as the value to be assigned
fits in both. (And if it doesn't, I trust you'll agree that the code is
broken...) Casting at the call is simply going to misbehave, very
nastily, if somehow the variable isn't of the width the function is
expecting.
My concern here, ultimately, is that a cast used in that fashion is
guaranteed to silence a compiler warning even when the warning is
telling you about serious trouble. Because of that, I consider such a
cast to be bad style, no matter how certain you are that it's okay in
a given case. Any C programmer who's dealt with portability issues will
stop and look twice or three times at it, and so you are distracting and
irritating the reader.
> I guess the proper solution in that case is to #define a datatype used
> for return codes. Is it really worth that for this, though?
Probably not, although I seem to recall we have done that elsewhere
(pg_ctl maybe?)
regards, tom lane