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. > 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. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR