Re: [vdr] *scanf %a, patches...
On 01.12.2012 21:14, Juergen Lock wrote:
In article 50ba09cc.2040...@flensrocker.de 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=300896view=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=300896view=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=300896view=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?
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),
or put your own version of sscanf() into the proposed oscompat.c and handle
%a properly in there.
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
___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] *scanf %a, patches...
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.2040...@flensrocker.de 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=300896view=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=300896view=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=300896view=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
Re: [vdr] *scanf %a, patches...
On Sun, Dec 02, 2012 at 08:58:31PM +0100, Juergen Lock wrote:
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.2040...@flensrocker.de you write:
Am 30.11.2012 11:32, schrieb Gerald Dachs:
Am 2012-11-30 10:17, schrieb Lars Hanisch:
[...] And the tools.c patch for the
cCharSetConv destructor looks like an actual bug, iconv_close()
shouldn't be called with (iconv_t)-1.
Ah sorry that patch isn't in ports yet, and now that I think of it
it only occured when testing the epgfixer plugin which doesn't quite
work properly yet anyway. But I guess it wouldn't hurt applying
anyway so here it is:
--- tools.c.orig
+++ tools.c
@@ -842,7 +842,8 @@ cCharSetConv::cCharSetConv(const char *F
cCharSetConv::~cCharSetConv()
{
free(result);
- iconv_close(cd);
+ if (cd != (iconv_t)-1)
+ iconv_close(cd);
}
void cCharSetConv::SetSystemCharacterTable(const char *CharacterTable)
Thanx again, :)
Juergen
___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
