Gerald Pfeifer wrote:
> We currently have the following code in tape.c:
>
> if (data->Offset.u.LowPart >= 0) {
> cmd.mt_op = MTFSF;
> cmd.mt_count = data->Offset.u.LowPart;
> }
> else {
> cmd.mt_op = MTBSF;
> cmd.mt_count = -data->Offset.u.LowPart;
> }
>
> data is of type TAPE_SET_POSITION*, which in turn is defined as
>
> typedef struct _TAPE_SET_POSITION {
> ULONG Method;
> ULONG Partition;
> LARGE_INTEGER Offset;
> BOOLEAN Immediate;
> } TAPE_SET_POSITION, *PTAPE_SET_POSITION;
>
> Offset is of type LARGE_INTEGER which is defined as
>
> struct {
> DWORD LowPart;
> LONG HighPart;
> } u;
>
> Note how LowPart is unsigned (DWORD) here, so indeed the comparisons
> for >= 0 is always going to evaluate to true.
>
> Hans, I believe you originally wrote that code. Do you remember what
> you where trying to do here?
>
> The patch below removes this dead code and is equivalent to what we
> currently have. I am not sure it is the desired way to fix what I
> describe above, though.
>
The LARGE_INTEGER type without #ifdefs for clarity:
typedef union _LARGE_INTEGER {
struct {
DWORD LowPart;
LONG HighPart;
} u;
LONGLONG QuadPart;
} LARGE_INTEGER, *PLARGE_INTEGER;
The type is meant to be wrap a signed integer, but the compiler
legitimately flagged that it isn't in the way that it is being accessed
currently. Therefore, I believe the code should be changed to this:
> if (data->Offset.QuadPart >= 0) {
> cmd.mt_op = MTFSF;
> cmd.mt_count = (int)data->Offset.QuadPart;
> }
> else {
> cmd.mt_op = MTBSF;
> cmd.mt_count = -(int)data->Offset.QuadPart;
> }
--
Rob Shearman
