On Sun, Dec 02, 2012 at 02:19:44PM +0100, Klaus Schmidinger wrote:
> On 01.12.2012 21:14, Juergen Lock wrote:
> > In article <50BA09CC.2040104@xxxxxxxxxxxxxx> you write:
> >> Am 30.11.2012 11:32, schrieb Gerald Dachs:
> >>> Am 2012-11-30 10:17, schrieb Lars Hanisch:
> >>>> Looks like the pointer returned by sscanf is not valid:
> >>>>
> >>>> 32: bool tComponent::FromString(const char *s)
> >>>> 33: {
> >>>> 34: unsigned int Stream, Type;
> >>>> 35: int n = sscanf(s, "%X %02X %7s %a[^\n]", &Stream, &Type,
> >>>> language, &description); // 7 = MAXLANGCODE2 - 1
> >>>> 36: if (n != 4 || isempty(description)) {
> >>>> 37: free(description);
> >>>> 38: description = NULL;
> >>>> 39: }
> >>>> 40: stream = Stream;
> >>>> 41: type = Type;
> >>>> 42: return n >= 3;
> >>>> 43: }
> >>>
> >>> From man sscanf:
> >>>
> >>> The GNU C library supports a nonstandard extension that causes the library to
> >>> dynamically allocate a string of sufficient size for input strings for the %s
> >>> and %a[range] conversion specifiers.
> >>>
> >>> This is the reason why it doesn't work with ulibc.
> >>
> >> Then there should be a malloc or something similiar for description:
> >>
> >> 32: bool tComponent::FromString(const char *s)
> >> 33: {
> >> 34: unsigned int Stream, Type;
> >> description = malloc(strlen(s));
> >> description[0] = 0;
> >> 35: int n = sscanf(s, "%X %02X %7s %a[^\n]", &Stream, &Type, language, &description); // 7 = MAXLANGCODE2 - 1
> >> 36: if (n != 4 || isempty(description)) {
> >> 37: free(description);
> >> 38: description = NULL;
> >> 39: }
> >> 40: stream = Stream;
> >> 41: type = Type;
> >> 42: return n >= 3;
> >> 43: }
> >>
> >> A check for description != NULL before the free call is not needed.
> >>
> >> But this is not the only place in the vdr code, where %a is used...
> >>
> > Btw FreeBSD has the same problem (no *scanf %a in libc), so you
> > could try taking the FreeBSD patches and replacing
> >
> > #ifdef __FreeBSD__
> >
> > by
> >
> > #if defined(__FreeBSD__) || defined(__UCLIBC__)
> >
> > in those parts handling %a here:
> >
> > http://svnweb.freebsd.org/ports/head/multimedia/vdr/files/patch-vdr-1.7.28_FreeBSD?revision=300896&view=markup
> >
> > A few plugins are affected too: (of those we have in FreeBSD ports)
> >
> > eepg:
> >
> > http://svnweb.freebsd.org/ports/head/multimedia/vdr-plugin-eepg/files/patch-eepg.c?revision=300896&view=markup
> >
> > infosatepg:
> >
> > http://svnweb.freebsd.org/ports/head/multimedia/vdr-plugin-infosatepg/files/patch-process.cpp?view=markup
> >
> > ttxtsubs:
> >
> > http://svnweb.freebsd.org/ports/head/multimedia/vdr-plugin-ttxtsubs/files/patch-ttxtsubschannelsettings.c?revision=300896&view=markup
> >
> > HTH, :)
> > Juergen
> >
> > PS: I think I never sent the FreeBSD patches to Klaus the last few
> > times I updated them, atm FreeBSD ports is behind again (still at
> > vdr 1.7.29), when I get around catching up to the latest version I
> > can send the updated patches if you are interested, Klaus?
>
> Well, in all honesty (and without meaning any offense or disrespect),
> I wouldn't like to have all these "#if ..." spread around the VDR source.
> For instance, things like a missing getline() function should not be
> patched right into the tools.c code, but rather be put into a separate
> file, maybe named oscompat.[hc]. In that file, depending on the OS in use,
> a local implementation of getline() could be implemented, and tools.c
> (like any other file that needs OS compatibility stuff) could just
> #include "oscompat.h" and the Makefile could link oscompat.o into
> the final executable.
>
> There are also some patches that don't, at first glance, look like they
> would be BSD specific, like those in plugin.c. Are these just a different
> way of handling things (and would this also work on Linux)? Or is this
> whole shebang handled differently in BSD?
>
Hm I don't remember where that particular patch came from but I
_think_ the issue there is just that on BSD like errno with syscalls,
dlerror() isn't reset when dlopen() or dlsym() are successful (return
non-NULL), and yes the idea is that the patch should work on Linux
too, which is why we didn't put in #ifdef __FreeBSD__. (Not sure
anyone actually tested this on Linux yet tho.)
> Well, finally for the root of this whole discussion: I really like the
> "%a" format character in sscanf(), because it is so simple and straightforward.
> I don't see why other libcs don't implement this, and I wouldn't want to
> have all that bulk in such areas. So I suggest you either get the libcs on
> those platforms to implement this (and thus solve the problem once and for all),
Uhm. :) %a surely isn't standardized anywhere so it would be
called a prime example of a Linuxism, even if it may be an useful
one. But you can't really expect everyone else to implement it
too...
> or put your own version of sscanf() into the proposed oscompat.c and handle
> "%a" properly in there.
>
Well, not sure when I would get to _that_... (As just taking glibc's
*scanf() will probably end up needing a lot of other code from glibc
too.) Guess I'll just leave those parts of the patches in FreeBSD
ports for now then, unless someone else comes up with code for this
before me.
Tho getline() btw is less important anyway, it's only missing on
FreeBSD 7.x where most of the ways to get dvb drivers don't work
yet so vdr would pretty much only be useful as a client instance
or for IPTV there. (_And_ FreeBSD 7.x will be EOL'd soon too.)
Oh and btw the receiver.c patch would affect Linux too if building
with llvm/clang, which ignores accesses thru a NULL pointer constant
and instead prints a warning. And the tools.c patch for the
cCharSetConv destructor looks like an actual bug, iconv_close()
shouldn't be called with (iconv_t)-1.
> > A few more 1.7.29 patches are in here:
> >
> > http://svnweb.freebsd.org/ports/head/multimedia/vdr/files/
> >
> > (I think I did send FreeBSD patches to those plugin maintainers
> > that may still be active some time ago, haven't heard back from
> > many tho.)
>
> Klaus
>
...and thanx for VDR! :)
Juergen
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
