(no subject)

[linguist]

This is just a random observation, I don't know this piece of code
or the kernel in general, but instinct tells me that where is says
"if (!fd) return -EBADF", it should say "if (!file) return -EBADF".
Just a heads up.

Regards,
Rich

static int tiocgdev(unsigned fd, unsigned cmd,  unsigned int *ptr) 
{ 

	struct file *file = fget(fd);
	struct tty_struct *real_tty;

	if (!fd)
		return -EBADF;
	if (file->f_op->ioctl != tty_ioctl)
		return -EINVAL; 
	real_tty = (struct tty_struct *)file->private_data;
	if (!real_tty) 	
		return -EINVAL; 
	return put_user(new_encode_dev(tty_devnum(real_tty)), ptr); 
} 

[PATCH] fix tiocgdev 32/64bit emul

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

linguist@masterlinkcorp.com wrote:
> This is just a random observation, I don't know this piece of code
> or the kernel in general, but instinct tells me that where is says
> "if (!fd) return -EBADF", it should say "if (!file) return -EBADF".
> Just a heads up.
> 
> Regards,
> Rich
> 
> static int tiocgdev(unsigned fd, unsigned cmd,  unsigned int *ptr) 
> { 
> 
> 	struct file *file = fget(fd);
> 	struct tty_struct *real_tty;
> 
> 	if (!fd)
> 		return -EBADF;

Yup, looks like a real bug to me...  good catch.

Untested but obvious patch attached.

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 370 bytes --]

===== arch/x86_64/ia32/ia32_ioctl.c 1.38 vs edited =====
--- 1.38/arch/x86_64/ia32/ia32_ioctl.c	Wed Feb 25 11:06:01 2004
+++ edited/arch/x86_64/ia32/ia32_ioctl.c	Sun Mar 21 11:43:56 2004
@@ -27,7 +27,7 @@
 	struct file *file = fget(fd);
 	struct tty_struct *real_tty;
 
-	if (!fd)
+	if (!file)
 		return -EBADF;
 	if (file->f_op->ioctl != tty_ioctl)
 		return -EINVAL; 

Re: [PATCH] fix tiocgdev 32/64bit emul

[Christoph Hellwig]

On Sun, Mar 21, 2004 at 11:45:12AM -0500, Jeff Garzik wrote:
> Yup, looks like a real bug to me...  good catch.
> 
> Untested but obvious patch attached.

Isn't that SuSE's strange ioctl hack that has been rejected for mainline
multiple times?  why does x86_64 have an emulation for it if the ioctl
isn't implemented anyway?

Re: [PATCH] fix tiocgdev 32/64bit emul

[Carl-Daniel Hailfinger]

On 2004-03-21 16:57:52, Christoph Hellwig wrote:
> On Sun, Mar 21, 2004 at 11:45:12AM -0500, Jeff Garzik wrote:
>> Yup, looks like a real bug to me...  good catch.
>>
>> Untested but obvious patch attached.
> 
> Isn't that SuSE's strange ioctl hack that has been rejected for mainline
> multiple times?  why does x86_64 have an emulation for it if the ioctl
> isn't implemented anyway?

Since this pops up from time to time, please let me explain what TIOCGDEV
does (if you know already, feel free to scoll to the end) and ask for
alternative solutions.

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=89051325309548
On 1998-03-21 20:41:37, Miquel van Smoorenburg wrote:
> This patch introduces a new ioctl, TIOCGDEV. It can be used to find
> out the _real_ device of the serial console.
> 
> I use this so I can first open /dev/console and a pty pair, then use
> TIOCCONS to redirect console output to the pty, and then I can copy
> the console output from the pty to both a logfile and the real console.
> This is useful for a boot-time daemon so all bootup messages can be
> captured and logged...

Further links to relevant messages:
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696542915335
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696701517101
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696788418144

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=102929921716152
On 2002-08-14 4:26:45, Linus Torvalds wrote:
> I've always hated the fact that all the boot-time messages get lost,
> simply because syslogd hadn't started, and as a result things like fsck
> ran without any sign afterwards.

http://marc.theaimsgroup.com/?l=linux-kernel&m=102931212924401
http://marc.theaimsgroup.com/?l=linux-kernel&m=103383136010219
http://marc.theaimsgroup.com/?l=linux-kernel&m=103383754113901
http://marc.theaimsgroup.com/?l=linux-kernel&m=103418708112077
http://marc.theaimsgroup.com/?l=linux-kernel&m=103771548020416
http://marc.theaimsgroup.com/?l=linux-kernel&m=103771746822896

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=97692840309876
On 2000-12-16 0:55:37, Kurt Garloff wrote:
> some applications do need to know where the console (/dev/console)
> actually maps to. For processes with a controlling terminal, you may see
> it in /proc/$$/stat. However, daemons are supposed to run detached (they
> don't want to get killed by ^C) and some processes like init or bootlogd
> do still need to be able to find out.
> 
> The kernel provides this information -- sort of:
> It contains the TIOCTTYGSTRUCT syscall which returns a struct. Of course,
> it changes between different kernel archs and revisions, so using it is
> an ugly hack. Grab for TIOCTTYGSTRUCT_HACK in the bootlogd.c file of the
> sysvinit sources. Shudder!
> 
> Having a new ioctl, just returning the device no is a much cleaner solution,
> IMHO. So, I created the TIOCGDEV, which Miquel suggests in his sysvinit
> sources. It makes querying the actual console device as easy as
> int tty; ioctl (0, TIOCGDEV, &tty);
> 
> Patches against 2.2.18 and 2.4.0-testX are attached.

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=97692912911233
On 2000-12-16 1:11:07, Linus Torvalds wrote:
> Please instead do the same thing /dev/tty does, namely a sane interface
> that shows it as a symlink in /proc (or even in /dev)

Can we do that in sysfs now? Back then, nobody implemented it for /proc
and it now already seems to exist as /sys/class/tty/console/dev

Does /sys/class/tty/console/dev have the semantics needed above?


Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [PATCH] fix tiocgdev 32/64bit emul

[Christoph Hellwig]

On Sun, Mar 21, 2004 at 07:50:23PM +0100, Carl-Daniel Hailfinger wrote:
> > Isn't that SuSE's strange ioctl hack that has been rejected for mainline
> > multiple times?  why does x86_64 have an emulation for it if the ioctl
> > isn't implemented anyway?
> 
> Since this pops up from time to time, please let me explain what TIOCGDEV
> does (if you know already, feel free to scoll to the end) and ask for
> alternative solutions.

Oh, I know what it does.  Have you ever looked at Al's rawconsole patch
that he coded up exactly in response to that hack?

ftp://ftp.linux.org.uk/pub/people/viro/ftp://ftp.linux.org.uk/pub/people/viro/X0-rawconsole-B5

Re: [PATCH] fix tiocgdev 32/64bit emul

[Carl-Daniel Hailfinger]

Christoph Hellwig wrote:
> On Sun, Mar 21, 2004 at 07:50:23PM +0100, Carl-Daniel Hailfinger wrote:
> 
>>>Isn't that SuSE's strange ioctl hack that has been rejected for mainline
>>>multiple times?  why does x86_64 have an emulation for it if the ioctl
>>>isn't implemented anyway?
>>
>>Since this pops up from time to time, please let me explain what TIOCGDEV
>>does (if you know already, feel free to scoll to the end) and ask for
>>alternative solutions.
> 
> 
> Oh, I know what it does.  Have you ever looked at Al's rawconsole patch
> that he coded up exactly in response to that hack?
> 
> ftp://ftp.linux.org.uk/pub/people/viro/X0-rawconsole-B5

No. It was never announced on lkml, so I couldn't know of its existence.

Andi, Miquel: is the rawconsole patch usable for blogd?

Christoph: Have you looked at my question regarding
/sys/class/tty/console/dev ?


Regards,
Carl-Daniel

Re: [PATCH] fix tiocgdev 32/64bit emul

[Christoph Hellwig]

On Sun, Mar 21, 2004 at 08:57:58PM +0100, Carl-Daniel Hailfinger wrote:
> Christoph: Have you looked at my question regarding
> /sys/class/tty/console/dev ?

No, I'll leave that to Greg.  If you want my 2 (Euro-) Cent I'd rather avoid
exposing a dev_t to userspace wherever possible.

Re: [PATCH] fix tiocgdev 32/64bit emul

[Carl-Daniel Hailfinger]

Christoph Hellwig wrote:
> On Sun, Mar 21, 2004 at 08:57:58PM +0100, Carl-Daniel Hailfinger wrote:
> 
>>Christoph: Have you looked at my question regarding
>>/sys/class/tty/console/dev ?
> 
> 
> No, I'll leave that to Greg.  If you want my 2 (Euro-) Cent I'd rather avoid
> exposing a dev_t to userspace wherever possible.

Understood. Especially since the recent upsizing of dev_t broke
applications trying to be too clever about dev_t. However, look at this:

# cat /sys/class/tty/console/dev
5:1

Does this major:minor textfile export address your concerns?


Regards,
Carl-Daniel

Re: [PATCH] fix tiocgdev 32/64bit emul

[Christoph Hellwig]

On Sun, Mar 21, 2004 at 09:17:00PM +0100, Carl-Daniel Hailfinger wrote:
> > No, I'll leave that to Greg.  If you want my 2 (Euro-) Cent I'd rather avoid
> > exposing a dev_t to userspace wherever possible.
> 
> Understood. Especially since the recent upsizing of dev_t broke
> applications trying to be too clever about dev_t. However, look at this:
> 
> # cat /sys/class/tty/console/dev
> 5:1
> 
> Does this major:minor textfile export address your concerns?

No.  I'd really love to avoid having any userspace (escept makedev/udev)
to use dev_t as anything but a cookie, i.e. trying to make sense from it.

Having a special raw, non-redirected console device as in Al's patch is
*much* cleaner.