Re: [PATCH] Stop printk printing non-printable chars

[matthew-lkml]

On Fri, Jun 18, 2004 at 11:32:52PM +0200, Jan-Benedict Glaw wrote:
> On Fri, 2004-06-18 21:53:55 +0100, matthew-lkml@xxxxxxxxxxxxxxxxxxxxx <matthew-lkml@xxxxxxxxxxxxxxxxxxxxx>
> wrote in message <20040618205355.GA5286@xxxxxxxxxxxxxxxxxxxxx>:
> > printk to even consider printing _any_ non-printable characters at all.
> 
> It's dandy if you pump out some data via serial link.

Is printk ever used to send anything out via a serial link? I assumed it
was only kernel log messages (that should really be fairly sane). Log
messages sent to serial printer, etc, don't want dodgy chars in them
that may mess up the printer, do they?

> 
> > It makes all characters out of the range 32..126 (except for newline)
> > print as a '?'.
> 
> I don't see why that's needed. I'd say let's better fix ACPI to put
> those strings as a hexdump or something like that.

Looking at the ACPI code (and not understanding it too well) it looks
like this data is retrieved from the BIOS, but is only printed here for
info and not actually used anywhere. In this case, I'd think there isn't
a lot of point checking for data correctness in the ACPI code. As
someone else pointed out, though, other things can cause the kernel log
to print nasty chars that are unwanted, so there should really be a
check here anyway.

> > +		if (p[0] != '\n' && (p[0] < 32 || p[0] > 126)) {
> 
> So you're ripping off something that could be a nice feature and place
> some slow path. By the way, why do you use 'p[0]' instead of '*p'?

The string has just been through the equivalent of sprintf, so I guess
this is hardly going to slow it down much more. Used p[0] to look
tidier, matching another "if" statement 5 lines up. In new patch used
*p, as it doesn't really matter.

Thanks,

-- 
Matthew
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/