On Monday 20 January 2020 12:31:16 Jan Kara wrote: > On Sun 19-01-20 13:49:44, Pali Rohár wrote: > > Hello! I looked at udf code which converts linux time to UDF time and I > > found out that conversion of timezone is incorrect. > > > > Relevant code from udf_time_to_disk_stamp() function: > > > > int16_t offset; > > > > offset = -sys_tz.tz_minuteswest; > > > > dest->typeAndTimezone = cpu_to_le16(0x1000 | (offset & 0x0FFF)); > > > > UDF 2.60 2.1.4.1 Uint16 TypeAndTimezone; says: > > > > For the following descriptions Type refers to the most significant 4 bits of this > > field, and TimeZone refers to the least significant 12 bits of this field, which is > > interpreted as a signed 12-bit number in two’s complement form. > > > > TimeZone ... If this field contains -2047 then the time zone has not been specified. > > > > As offset is of signed 16bit integer, (offset & 0x0FFF) result always > > clears sign bit and therefore timezone is stored to UDF fs incorrectly. > > I don't think the code is actually wrong. Two's complement has a nice > properly that changing type width can be done just by masking - as an > example -21 in s32 is 0xffffffeb, if you reduce to just 12-bits, you get > 0xfeb which is still proper two's complement encoding of -21 for 12-bit wide > numbers. Ou right, so it is really OK. Thank you for clarification. > > This needs to be fixed, sign bit from tz_minuteswest needs to be > > propagated to 12th bit in typeAndTimezone member. > > > > Also tz_minuteswest is of int type, so conversion to int16_t (or more > > precisely int12_t) can be truncated. So this needs to be handled too. > > tz_minuteswest is limited to +-15 hours (i.e., 900 minutes) so we shouldn't > need to handle truncating explicitely. Fine, then code is correct. -- Pali Rohár pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: PGP signature