tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] Small patch


From: Thomas Preud'homme
Subject: Re: [Tinycc-devel] Small patch
Date: Thu, 31 Jan 2013 13:36:32 +0100
User-agent: KMail/1.13.7 (Linux/3.2.0-4-amd64; KDE/4.8.4; x86_64; ; )

Le jeudi 31 janvier 2013 13:19:59, grischka a écrit :
> Thomas Preud'homme wrote:
> >>  bcheck.o : lib/bcheck.c
> >> 
> >> -       gcc -c $< -o $@ $(CPPFLAGS) $(CFLAGS)
> >> +       $(CC) -c $< -o $@ $(CPPFLAGS) $(CFLAGS)
> > 
> > Unfortunetely it still breaks because of a wrong detection of GCC_MAJOR.
> > Because the test to determine the major version number of gcc fails,
> > GCC_MAJOR is set to 2. I made the following patch but I'm still not sure
> > how to handle flags when the compiler is not gcc.
> > 
> > Shall no flag be set and the project fail because of missing -I for
> > instance?
> > 
> > I don't have a clear idea for now.
> 
> Maybe we should just put bcheck.o into libtcc1.a (and thus make via
> the rules of lib/Makefile).  Unless there is a reason why we
> shouldn't.

Yes but that doesn't change the general assumption that we are compiling tcc 
with gcc. If --cc is specified at configure time, then CFLAGS and CPPFLAGS 
should be set when running make (make CPPFLAGS=foo CFLAGS=bar)

> 
> > -            h = elf_hash(name) % nbuckets;
> > +            h = name ? elf_hash(name) % nbuckets : 0;
> 
> This is counterproductive IMO because when you read this you get
> the wrong impression that there are cases where "name" is NULL.
> But in fact there is no such case.  So the naive reader is misled,
> while the knowledgeable reader must conclude that the writer didn't
> know what s/he was doing.

Well, the test at the top of the function suggested me name could be NULL. I 
suppose it can't be NULL when the symbol is global or common. What about a 
malformed elf file?

> 
> >              boolean.t = VT_BOOL;
> > 
> > +            boolean.ref = 0;
> 
> Also redundant because type.ref is used with type.t = VT_FUNC only.
> 
> I'm not completely opposed to make this look more solid but then
> such patch should address the problem systematically such that the
> code becomes smaller, not bigger.

Can you develop about what you have in mind?

> 
> > -                parse_btype(&btype, &ad);
> > +                if (parse_btype(&btype, &ad))
> > +                    expect("type");
> 
> Breaks tcc.  Never push without test, in particular not during the
> "calm down period" before release. ;)

Yes, you are absolutely right. I tried on a simple example (where tcc fails to 
issue a warning) and I was obviously wrong. You were right from the beginning, 
I shall stop doing any commits unless a very serious bug is reported and keep 
things for after the release.

> 
> >     > -                strcpy(buf, "__bound_");
> >     > -                strcat(buf, name);
> >     > +                snprintf(buf, sizeof(buf), "__bound_%s", name);
> 
> There are cases where we might want to use "pstrcpy" instead of "strcpy".
> This is no such case because "__bound_memcpy" cannot overflow buf[32].
> Same with
>       pstrcpy(buf, sizeof buf, "a.out");
> Because "a.out" cannot overflow buf[1024].

sprintf could be used  then to save some space

> 
> --- grischka

Thomas

Attachment: signature.asc
Description: This is a digitally signed message part.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]