Re: [Info-mtools] Bug: mformat does not format FAT32 correctly

[Alain Knaff]

On 2018-09-24 10:23, Pali Rohár wrote:
[...]
> Hm.. there are no messages printed to stdout. And also there are no
> debugging code.

I'm speaking about your patch from August 11th 15:42, which does indeed 
print messages to stdout (see the lines just before and after the call 
to get_lba_geom).

+                       printf("sector size: %u, sectors: %u, heads: %u, 
tracks: %u\n",                                                      
+                               128 << (used_dev.ssize & 0x7f), 
used_dev.sectors, used_dev.heads, used_dev.tracks);                          



> Those messages are printed to string buffer (sprintf)
> and contains errors. In same way how other functions do it.

That's how it was in 4.0.18, but your patch changes the sprintf(errmsg, 
...) to printf(...) to stdout.

-                       sprintf(errmsg,                                         
                                                             
-                               "Unknown geometry "                             
                                                             
-                               "(You must tell the complete geometry "         
                                                             
-                               "of the disk, \neither in /etc/mtools.conf or " 
                                                             
-                               "on the command line) ");                       
                                                             
-                       continue;                                               
                                                             
+                       printf("%s: "                                           
                                                             
+                               "Complete geometry of the disk was not 
specified, \n"                                                        



Ok, maybe that wasn't the version you intended to send me? That's why I 
asked you to doublecheck, and send me it again if ever you've 
accidentally sent me the wrong version. Mistakes do happen, that's not a 
problem, but right now I worry that there might be other issues in the 
version I got which might be less easy to spot than the error messages...

> 
>> Also, there is some puzzling "late capping" of sector_size which should
>> be unneeded (ssize is already being capped at input), and which would
>> introduce inconsistencies if for some reason it did indeed kick in.
> 
> No. ssize (= sector size) is no set correctly from block device.

... and that's where it should be capped, and not late in the game where 
other code parts may already have used the initial "too high" setting. 
The risk here is inconsistencies, where in some parts of the code assume 
a sector size bigger than 4K, and others 4K. And another risk is that it 
breaks those situations (2m floppies) where a bigger size than 4K _is_ 
acceptable.

[...]

Regards,

Alain