From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-lb0-x22e.google.com ([2a00:1450:4010:c04::22e]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZkWwU-0002hT-SZ for barebox@lists.infradead.org; Fri, 09 Oct 2015 12:38:35 +0000 Received: by lbbwt4 with SMTP id wt4so79161409lbb.1 for ; Fri, 09 Oct 2015 05:38:12 -0700 (PDT) Date: Fri, 9 Oct 2015 15:40:37 +0300 From: Peter Mamonov Message-ID: <20151009154037.389bb1ab@berta> In-Reply-To: <20151009080624.GF7858@pengutronix.de> References: <1444241036-23622-1-git-send-email-pmamonov@gmail.com> <20151009080624.GF7858@pengutronix.de> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [RFC] common: filetype: is_fat_or_mbr() considered harmful To: Sascha Hauer Cc: barebox@lists.infradead.org On Fri, 9 Oct 2015 10:06:24 +0200 Sascha Hauer wrote: > Hi Peter, > > On Wed, Oct 07, 2015 at 09:03:56PM +0300, Peter Mamonov wrote: > > Deleted pieces of code detect MBR-containig device as a FAT-type > > device, if it's first partition contains a FAT filesystem. So, one > > can mount the first partition of a hard drive containing FAT FS > > using the following command: barebox: mount /dev/ata0.0 /mnt/0 > > as well as this one: > > barebox: mount /dev/ata0 /mnt/1 > > Both commands mount the same FS. > > > > This behaviour causes automount (mount -a) to mount FAT FS > > on a first partition twice: > > barebox: mount > > none on / type ramfs > > none on /dev type devfs > > /dev/ata0 on /mnt/ata0 type fat > > /dev/ata0.0 on /mnt/ata0.0 type fat > > /dev/ata0.1 on /mnt/ata0.1 type ext4 > > This is_fat_or_mbr mechanism never worked very well and had funny side > effects. Would be nice to get rid of it. > Simply removing this option is not a solution though, we have to find > a proper way to keep the current feature and make it more sane. Ok, the patch comment is misleading a bit. I do not propose to get rid of the is_fat_or_mbr() completely. However, I do not see the point to check for a FAT FS, after the device was correctly detected as an MBR-type device: enum filetype file_name_detect_type(const char *filename) ... type = file_detect_type(buf, ret); if (type == filetype_mbr) { /* * Get the first partition start sector * and check for FAT in it */ is_fat_or_mbr(buf, &bootsec); ret = lseek(fd, (bootsec) * 512, SEEK_SET); if (ret < 0) goto err_out; ret = read(fd, buf, 512); if (ret < 0) goto err_out; type = is_fat_or_mbr((u8 *)buf, NULL); } The deleted code snippet was introduced by this patch: commit 010ee209b75c5732ae4144e3ee9ce14158193c1f Author: Franck Jullien Date: Wed Sep 19 13:09:01 2012 +0200 filetype: Improve FAT detection We may have some disk with MBR as a first sector. In this case, the current FAT check returns an error. However, the FAT sector exist and the MBR can tell us where it is. This patch add to file_name_detect_type function the ability to find the FAT boot sector on the first sector of the first partition in case it is not on sector 0. It also introduce is_fat_or_mbr to check if a buffer is a FAT boot or MBR sector Signed-off-by: Franck Jullien Signed-off-by: Sascha Hauer According to the patch message it was introduced to workaround FAT detection. However, after deletion of the code I'm still able to detect and mount FAT-containig partiotions. Peter > > Sascha > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox