2012-02-03 09:56:02

by Roland Eggner

[permalink] [raw]
Subject: Re: [PATCH] [trivial] fs/vfat: add *.cmd to filenames listed as executable by mount option "showexec"

On 2012-02-03 Fr 16:41:16 +0900, OGAWA Hirofumi wrote:
> Roland Eggner <[email protected]> writes:
>
> > The mount option "showexec" provides a somewhat more native view of vfat
> > filesystems: only filenames *.exe *.com *.bat are listed as executable, rather
> > than all files. This patch adds filenames *.cmd to those listed as executable.
> > *.cmd are usually scripts associated with the shell "cmd.exe", which has been
> > the default shell in several OS versions of its vendor.
> >
> > Signed-off-by: Roland Eggner <[email protected]>
> > ---
>
> Hm, this may be good for someone, but this may not be for someone. So,
> this probably should be option if someone really care it.
>
> I'm not thinking about how though...
>
Think of VM images of recent versions of the FAT originating OS: currently
"showexec" will miss roughly half of the files, for which it is intended for.
IMHO this patch provides a very cheap completion of a feature of the vfat kernel
module. Cheap in terms of only 9 characters additional source code.

If you really see an advantage in the current, incomplete state of the
"showexec" feature, compared to the proposed, completed state: please elaborate
your objection.

> > --- a/Documentation/filesystems/vfat.txt 2011-10-24 09:10:05.000000000 +0200
> > +++ b/Documentation/filesystems/vfat.txt 2012-02-03 07:26:30.457441230 +0100
> > @@ -114,7 +114,7 @@
> >
> > showexec -- If set, the execute permission bits of the file will be
> > allowed only if the extension part of the name is .EXE,
> > - .COM, or .BAT. Not set by default.
> > + .COM, .BAT or .CMD. Not set by default.
> >
> > debug -- Can be set, but unused by the current implementation.
> >
> > --- a/fs/fat/inode.c 2012-02-03 06:23:35.721409360 +0100
> > +++ b/fs/fat/inode.c 2012-01-23 01:35:29.000000000 +0100
> > @@ -326,7 +326,7 @@ struct inode *fat_iget(struct super_bloc
> >
> > static int is_exec(unsigned char *extension)
> > {
> > - unsigned char *exe_extensions = "EXECOMBAT", *walk;
> > + unsigned char *exe_extensions = "EXECOMBATCMD", *walk;
> >
> > for (walk = exe_extensions; *walk; walk += 3)
> > if (!strncmp(extension, walk, 3))

--
Roland Eggner <[email protected]>


Attachments:
(No filename) (2.20 kB)
(No filename) (198.00 B)
Download all attachments

2012-02-03 12:51:39

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH] [trivial] fs/vfat: add *.cmd to filenames listed as executable by mount option "showexec"

Roland Eggner <[email protected]> writes:

> On 2012-02-03 Fr 16:41:16 +0900, OGAWA Hirofumi wrote:
>> Roland Eggner <[email protected]> writes:
>>
>> > The mount option "showexec" provides a somewhat more native view of vfat
>> > filesystems: only filenames *.exe *.com *.bat are listed as
>> > executable, rather
>> > than all files. This patch adds filenames *.cmd to those listed
>> > as executable.
>> > *.cmd are usually scripts associated with the shell "cmd.exe",
>> > which has been
>> > the default shell in several OS versions of its vendor.
>> >
>> > Signed-off-by: Roland Eggner <[email protected]>
>> > ---
>>
>> Hm, this may be good for someone, but this may not be for someone. So,
>> this probably should be option if someone really care it.
>>
>> I'm not thinking about how though...
>>
> Think of VM images of recent versions of the FAT originating OS: currently
> "showexec" will miss roughly half of the files, for which it is intended for.
> IMHO this patch provides a very cheap completion of a feature of the vfat kernel
> module. Cheap in terms of only 9 characters additional source code.
>
> If you really see an advantage in the current, incomplete state of the
> "showexec" feature, compared to the proposed, completed state: please elaborate
> your objection.

You would be happy with adding "cmd", and may think enough. But Someone
is not happy with it. And someone want to add "sh", "py", and "pl".

If I have to tackle it, why I have to make happy only your state? I'm
not saying current has advantage, I'm saying your patch doesn't make
guys happy like current one. It is just moving a place of happiness from
a group to an another group, right?

Thanks.
--
OGAWA Hirofumi <[email protected]>