Re: Dead code in dlls/ntdll/tape.c

[Robert Shearman]

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;
>     }