[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Tinycc-devel] Several patchs from Debian packaging
From: |
Thomas Preud'homme |
Subject: |
Re: [Tinycc-devel] Several patchs from Debian packaging |
Date: |
Sun, 18 Apr 2010 21:45:38 +0200 |
User-agent: |
KMail/1.12.4 (Linux/2.6.32-3-686; KDE/4.3.4; i686; ; ) |
On Saturday 17 April 2010 22:59:55 grischka wrote:
> Thanks for your work. Some comments on the pushed patches:
>
> RoboTux wrote:
> > [P|8de9b7a] Correctly support all unary expression with sizeof
> > Unary expression can start with a parenthesis. Thus, the current test
> > to detect which sizeof form is being parsed is inaccurate. This patch
> > makes tcc able to handle things like sizeof (x)[1] where x is declared
> > as char x[5]; wich is a valid unary expression
>
> "vtop_saved" is never used.
Deleted
>
> > [P|47abdbd] Better handle ld scripts
> > * search file from INPUT and GROUP commands in the library path in
> > addition to the current directory
> > * handle libraries specified by -lfoo options
> > * Search lib in GROUP command repeatedly
>
> +static int new_undef_sym = 0;
Moved on top of tccelf.c
>
> Variables (in particular static ones) should be in the file where
> they are used. tcc.h should have prototypes only.
>
> + ext = strrchr(base, '.');
>
> We have "tcc_fileextension(name)" for that.
Changed to:
ext = tcc_fileextension(filename);
if (*ext == '\0')
return 0;
>
> + snprintf(filename, strlen(libname) + 5, "%s.def", libname);
>
> It is pointless to use snprintf this way. Use just sprintf.
>
> static int filename_to_libname(TCCState *s1, char filename[], char
> libname[])
>
> "libname" should be const (because it is "input only").
You mean filename in that case, and libname for libname_to_filename I guess.
>
> *(--ext) = '\0';
> strcpy(libname, filename);
> *ext = '.';
>
> That's ugly.
Ok, I changed to:
if (libprefix && (!strncmp(ext, ".so", 2))) {
size_t len = ext - filename - 3;
strncpy(libname, filename + 3, len);
*(libname + len) = '\0';
return 1;
}
for the 3 tests (minus the - 3 for the ".def" comparison when TCC_TARGET_PE is
defined).
I also wonder wether is strncmp is a good idea here as it could match .sowtf
for example. I think strcmp is more appropriate here, don't you think ?
>
> + if (ret)
> + goto free_bufstate;
> + while (!ret && new_undef_syms()) {
> + tcc_load_buffer_state(buf_state, off);
> + ret = ld_add_file_list(s1, 0);
> + }
> + free_bufstate:
>
> The first two lines here are redundant.
>
> In any case the "save_buffer_state" idea that abuses the reader to
> resolve forward library symbols is horrible. Maybe you can put the
> library-names in a list (aka dynarray) and then just loop over that
> list.
In fact I didn't do that before because I wanted to avoid multiple libname to
filename and filename to libname conversion. But it turns to be to heavy that
way.
I'm working on that right now and then I commit the result (probably tomorrow
as I'm getting tired)
>
> > [P|e9406c0] Complain for static fct declared w/o file scope
> > Error out on static function without file scope and give an explaination
> > to the user
>
> Probably too correct. It's not an error with GCC and also breaks
> compilation of some older code I use for testing.
Could you provide me an example where this is the case ? (Cf my previous
answer)
>
> --- grischka
>
Thomas Preud'homme
- [Tinycc-devel] Several patchs from Debian packaging, RoboTux, 2010/04/17
- Re: [Tinycc-devel] Several patchs from Debian packaging, grischka, 2010/04/17
- Re: [Tinycc-devel] Several patchs from Debian packaging,
Thomas Preud'homme <=
- Re: [Tinycc-devel] Several patchs from Debian packaging, Thomas Preud'homme, 2010/04/18
- Re: [Tinycc-devel] Several patchs from Debian packaging, grischka, 2010/04/18
- Re: [Tinycc-devel] Several patchs from Debian packaging, RoboTux, 2010/04/20
- Re: [Tinycc-devel] Several patchs from Debian packaging, grischka, 2010/04/20
- Re: [Tinycc-devel] Several patchs from Debian packaging, RoboTux, 2010/04/20
- Re: [Tinycc-devel] Several patchs from Debian packaging, grischka, 2010/04/20
- [Tinycc-devel] Cleaning mob [Was Several patchs from Debian packaging], RoboTux, 2010/04/20
- [Tinycc-devel] Re: Cleaning mob [Was Several patchs from Debian packaging], grischka, 2010/04/21
- Re: [Tinycc-devel] Re: Cleaning mob [Was Several patchs from Debianpackaging], Timo VJ Lähde, 2010/04/21
- Re: [Tinycc-devel] Re: Cleaning mob [Was Several patchs from Debianpackaging], RoboTux, 2010/04/21