rewriting config_*, xmalloc_fget*

Denys Vlasenko vda.linux at googlemail.com
Fri Jun 17 01:44:45 UTC 2011


On Friday 17 June 2011 03:00, Denys Vlasenko wrote:
> On Thursday 16 June 2011 06:51, Timo Teräs wrote:
> > >> Though, in bb config_* API, getdelim isn't always enough. They seem to
> > >> need a special line reader that treats both \0 and \n as line terminator.
> > > 
> > > Why? \0 is not valid in text files and certainly shouldn't appear in
> > > config files... What is it needed for?
> > 
> > Well, that's the way they just work upstream, and you get regression if
> > you try to change it. I agree on that most applets do not need it, and
> > might actually be sort of broken for treating \0 as new line now.
> > 
> > Notably, though, at least sed.c seems to rely on the fact that \n and \0
> > are both recognized, and jumps through many hoops to handle them both
> > properly. Apparently there's out some sed scripts that use both \n and
> > \0 as statement separator, but they have different meaning.
> 
> sed does not use bb_get_chunk_with_continuation() directly.
> It uses bb_get_chunk_from_file(), which is implemented with
> bb_get_chunk_with_continuation().
> 
> bb_get_chunk_with_continuation() is used only by config parser,
> and config parser definitely has no need to deal with \0.
> Meaning: it needs to not segfault or hand on files with \0,
> but other than this, it is free to either ignore \0, or replace
> then with space or newline. IOW: whoever has \0 in their *config*
> files is shooting himself in the foot.
> 
> The first step would be to unmarry these two functions.

Done in this commit:

http://git.busybox.net/busybox/commit/?id=a1a448347e71c9899ad1500cbd8739fd82e1bb91

bb_get_chunk_with_continuation -> get_line_with_continuation,
become static in parse_config.c,
lost one parameter and one realloc per line.

Since now it is nicely localized to parse_config.c, we can optimize it for speed
further. For example, we can avoid realloc per each line by reusing the same
buffer.

As to getc_unlocked, well... we can do better that using it only here.
NONE of bbox applets are threaded, does it mean we can simply use
foo_unclocked everywhere by #define magic?

-- 
vda


More information about the busybox mailing list