On 23.05.2011 20:14, Warner Losh wrote: > Since we don't have a good review tool for the project, this may seem a > little fragmented, but here goes: > > + if (pp->sectorsize > 4096) > + return (EOPNOTSUPP); > > What's the motivation for this? This seems unrelated to the problem at > hand. It may be good, but 4096-byte sector size is very new and I'm not > sure MS-DOS can cope... :) There has been a long tradition in the MBR, > however, of supporting different sector sizes to allow old-old-old > school floppies that had 128 or 256 byte sectors to work, as well as > allowing the supper-compressed 1.77MB floppies to work with their > 1024-byte (or was that 2048-byte?) sectors. Not sure this really > matters for reading, and for writing this case is so far beyond the edge > I'm not even sure why I bring it up[*]
We have similar check in the g_part_mbr_probe function, and since we do not support sectorsize > 4096 in the probe method, i do not think we should try to create MBR for these providers. > In any event, I'd be tempted to use a #define for 4096 like > MBR_MAX_SECTOR_SIZE. > > - msize = MIN(pp->mediasize / pp->sectorsize, UINT32_MAX); > + msize = MIN(pp->mediasize / pp->sectorsize, 2 * UINT32_MAX); > > Why this change? I think that it is in two places. Currently we have limit to msize = UINT32_MAX, but partition in MBR has start offset and size (not end offset). Theoretically it can have size that is up to UINT32_MAX sectors, also start offset can be UINT32_MAX. And for example, for 4T disk we can have 2 partitions with 2TB size. -- WBR, Andrey V. Elsukov
signature.asc
Description: OpenPGP digital signature
