Re: [Tinycc-devel] Several patchs from Debian packaging

[grischka]

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.

> [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;

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.

  +        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").

  *(--ext) = '\0';
  strcpy(libname, filename);
  *ext = '.';

That's ugly.

  +                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.

> [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.

--- grischka