Notes |
(0002419)
kiltedknight
05-30-07 09:46
|
The problem is the off_t value. For it to work, it must be a 64-bit number and lseek64() must be used.
The attached patch will correct the problem. |
| |
(0002420)
kiltedknight
05-30-07 10:01
|
Of note, the fdisk.c found in the Fedora Core 6 util-linux RPM does not use off_t to define the various offsets. It defines them all as "unsigned long long" instead, as it is highly unlikely that you will ever find a hard drive that will no longer have its offsets fit within 32 bits. |
| |
(0002421)
kiltedknight
05-30-07 10:07
|
Use the updated fix, as it more closely mimics the base fdisk.c file, only casting to off_t in seek_sector(). |
| |
(0002422)
vda
05-31-07 16:13
|
- off_t offset = secno * sector_size;
+ off_t offset = (off_t) secno * sector_size;
Doesn't look right to me. |
| |
(0002423)
vda
05-31-07 16:15
|
static void
-seek_sector(off_t secno)
+seek_sector(unsigned long long secno)
{
- off_t offset = secno * sector_size;
+ off_t offset = (off_t) secno * sector_size;
if (lseek(fd, offset, SEEK_SET) == (off_t) -1)
fdisk_fatal(unable_to_seek);
}
I mean, you need to use lseek64 instead, and also need to make sure fd is opened with O_LARGEFILE. |
| |
(0002424)
vda
05-31-07 16:16
|
%"OFF_FMT"ld and %"OFF_FMT"lu are wrong. Use %lld, %llu. |
| |
(0002425)
bernhardf
06-01-07 01:50
|
no, %lld and %llu are wrong. For very obvious reasons, there is an inttypes.h that defines the proper printf format specifiers.
Use PRId64 and PRIu64
For odd platforms, we have fixup defines for these printf specifiers in platform.h, fwiw.
thanks, |
| |
(0002426)
kiltedknight
06-01-07 09:33
|
- off_t offset = secno * sector_size;
+ off_t offset = (off_t) secno * sector_size;
That's exactly what's in the fdisk.c file that I just uploaded.
On linux (Fedora Core 6), lseek()'s second parameter is defined as an off_t... which in this case is an unsigned long long. All other places where busybox has an off_t, the util-linux-2.13-pre7 package's fdisk.c has unsigned long long.
Granted, if you turn on the busybox option to use 64-bit sizes for files, this issue goes away... but in the case of a hard disk, how often do you come across one that is less than 2GB nowadays? |
| |
(0002436)
vda
06-02-07 04:32
|
But, bernhard, the patch prints long longs with %"OFF_FMT"ld! This cannot be right... |
| |
(0002438)
bernhardf
06-02-07 05:59
|
ok. off_t != long long.
To print an off_t, use printf("%"PRId64), my_offset); if sizeof(off_t) == sizeof(long long), else the respective format from inttypes.h
Introducing an PRIoff_t with appropriate checks in platform.h should make this work universally on 128, 64, 32, 16 bit arches and others.
Sounds ok? |
| |
(0002439)
vda
06-02-07 06:10
|
Fixed in svn, just after 1.6.0 release :(
With CONFIG_LFS=y:
function old new delta
unable_to_read - 20 +20
unable_to_write - 19 +19
unable_to_seek - 18 +18
unable_to_open - 15 +15
get_boot 1746 1759 +13
wrong_p_order 140 138 -2
verify 1116 1114 -2
set_partition 561 557 -4
add_partition 2389 2357 -32
.rodata 127000 126968 -32
fdisk_fatal 112 60 -52
------------------------------------------------------------------------------
(add/remove: 4/0 grow/shrink: 1/6 up/down: 85/-124) Total: -39 bytes
With CONFIG_LFS not set:
function old new delta
add_partition 1485 2357 +872
verify 811 1114 +303
set_partition 264 557 +293
list_table 725 869 +144
fdisk_main 3194 3272 +78
get_boot 1712 1759 +47
fill_bounds 135 182 +47
wrong_p_order 108 138 +30
delete_partition 428 450 +22
unable_to_read - 20 +20
unable_to_write - 19 +19
unable_to_seek - 18 +18
unable_to_open - 15 +15
seek_sector 44 59 +15
create_doslabel 137 147 +10
add_logical 119 126 +7
extended_offset 4 8 +4
write_table 257 260 +3
.rodata 126840 126808 -32
fdisk_fatal 112 60 -52
------------------------------------------------------------------------------
(add/remove: 4/0 grow/shrink: 14/2 up/down: 1947/-84) Total: 1863 bytes |
| |
bb-fdisk-fix.diff [^] (5,375 bytes) 05-30-07 09:46
