[vdr] Re: [PATCH] fix segfaults because of channels with umlauts

[Ludwig Nussel <ludwig.nussel@gmx.de>]

Klaus Schmidinger wrote:
> Ludwig Nussel wrote:
> > 
> > Klaus Schmidinger wrote:
> > > Klaus Schmidinger wrote:
> > > >
> > > > Ludwig Nussel wrote:
> > > > >
> > > > > Nevertheless it's a bug in vdr to ignore the
> > > > > return value and to not check for NULL. Attached patch is for 1.2.6,
> > > > > 1.3.x needs a similar one.
> > > >
> > > > I guess you're right about checking the return value of asprintf().
> > > > But first I'd like to know exactly why asprintf() failed on your system.
> > > > The typical error mentioned in the GNU C Library Reference Manual for
> > > > asprintf() would be a failure to allocate the buffer, which normally
> > > > should be pretty unlikely, given the amount of RAM today's systems have...
> > >
> > > I've decided against checking all the return values of asprintf(), because
> > > that would be too much hassle in many places. Besides, there are many places
> > > where objects are allocated on the heap and it is not explicitly checked
> > > whether the returned pointer is valid. At some point one must be able to
> > > assume that there is enough memory available, otherwise the whole system
> > > wouldn't be able to work reasonably at all.
> > 
> > The problem here is not failure to allocate memory but as the
> > manpage says 'some other error'. Invalid UTF-8 strings are probably
> > just one example of 'some other error'.
> 
> Well, there haven't been any "other errors" in years, its just that nasty
> UTF-8 that some distributions now apparently have made the default. Personally
> I don't see why my system should use UTF-8. I am German, all right, but iso8859-1
> works just fine for me, so no need for the hassle with those two-byte characters.

I'm sure the same can be said if you are czech, greek or turkish and
use their language specific encoding.

> > Ignoring that leads to
> > strange crashes. The quick workaround I posted does just #define
> > asprintf to something that sets the pointer to NULL in case of
> > failure. If vdr then crashes because of a NULL pointer it's clearly
> > a bug and easy to debug. Instead of the #define it would also be
> > possible to write a real function with a different name as wrapper
> > to asprintf or to put the wrapper into a different namespace like
> > vdr::asprintf and substitute all asprintf with that.
> 
> What I could agree with is to always initialize the given buffer to NULL
> (I believe there are some places where this isn't the case). However, this
> won't help if you have UTF-8 turned on, because asprintf() will fail, anyway.

Even worse, it may modify the pointer to make it invalid. 'If memory
allocation wasn't possible, or some other error occurs, these
functions will return -1, and the contents of strp is undefined'.
Relying on undefined behaviour will bite sooner or later.

> > > The actual problem here doesn't even have to do with memory consumption,
> > > but rather with incompatible character representations, which would cause
> > > the program to work wrong, anyway. So I'd rather remove the root of all evil,
> > > which is UTF-8. I'll therefore add a few lines to vdr.c, which will check
> > > this at program startup and exit if necessary:
> > 
> > That's nasty, better downgrade it to a warning.
> 
> As we can see it just won't work with UTF-8, so I'll keep it a fatal error.
> Better stop right away than let people run into strange crashes.

It won't crash with the patch applied but instead display "???".

> > A proper fix would
> > be to convert all incoming strings to the encoding vdr runs in
> > (which ideally would be UTF-8 to be able to handle all languages).
> 
> Which would also require a lot more extensive fonts, I believe.

That's another story. You can still use UTF-8 internally even though
the ttpci osd can't display all (most) characters. Other output
devices might be more powerful. Think e.g. of Browsers accessing the
EPG via vdradmin.

> Besides, as far as I can see the stations on Astra all use some
> iso8859-x, so why bother with UTF-8?

I don't know if there are czech, greek or turkish channels, maybe on
other satellites. There's at least Al Jazeera but that doesn't have
EPG.

> > Btw, right now vdr creates invalid directories in /video when the
> > system locale is UTF-8 which means that e.g. gtk2 programs don't
> > display them in the file selection dialog.
> 
> Well, looks like we'll need yet another make option like VFAT=1.
> I can either map all 8-bit characters to some 7-bit representation
> (as, for instance, mapping 'ä' to 'ae') or leave them out entirely.

Please, no make options. That just doesn't work if you want to
package vdr. Leaving them out of the filename is a good idea though.
Store the information in a file instead (in UTF-8 of course ;-)).

> I'm not going to implement UTF-8 support in any way, because I
> consider UTF-8 one of the most stupid ideas in computer history.
> To me a character is a character is a character. It is ONE entity,
> not sometimes one and sometimes two. If we really need more characters
> at the same time, we'll need to use 16 or 32 bit per single character.

THAT would mean real trouble as a char* wouldn't work anymore
because of null bytes inside the string.

Anyway, I don't demand you to support UTF-8. Just prevent vdr from
crashing because of stupid asprintf behaviour. As long as you don't
use any locale aware functions from e.g. glibc you don't need to
care much about UTF-8. It's just a null terminated block of bytes
after all.

cu
Ludwig

-- 
(o_  Ludwig.Nussel@gmx.de
//\  PGP Key ID: FF8135CE
V_/_ ICQ:        52166811