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

[Pali Rohár]

On Monday 24 September 2018 10:47:46 Alain Knaff wrote:
> 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?

It is correct version. I forgot that I put those messages on stdout.
Sorry for that.

Anyway, I thought that it would be useful message for end-user that
autodetection of parameters did not work and that some calculation was
done. So it print parameters which it then use.

So do you think that those messages about CHS geometry should not be
printed?

> 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.

So... where in the code it should be? I looked at the code and I thought
that correct place for BLKSSZGET is in get_block_geom().

... Or when talking about 4K, do you mean check "Fs.sector_size > 4096"?
Because there are two different things, one is file system sector size
and one block device sector size.

If there are devices/systems which uses "Fs.sector_size > 4096", then
that check should not be introduced.

But I was told that such devices/systems do not exists and FAT sector
size is maximally 4096.

> [...]
> 
> Regards,
> 
> Alain
> 

-- 
Pali Rohár
address@hidden