Dead code in dlls/ntdll/tape.c

Robert Shearman rob at codeweavers.com
Thu Nov 1 16:16:07 CDT 2007


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




More information about the wine-devel mailing list