[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
signature.asc
Description: This is a digitally signed message part.
- Re: [Tinycc-devel] Small patch, (continued)
- Re: [Tinycc-devel] Small patch, Thomas Preud'homme, 2013/01/31
- Re: [Tinycc-devel] Small patch, Thomas Preud'homme, 2013/01/31
- Re: [Tinycc-devel] Small patch, Vincent Lefevre, 2013/01/31
- Re: [Tinycc-devel] Small patch, grischka, 2013/01/31
- Re: [Tinycc-devel] Small patch, Vincent Lefevre, 2013/01/31
- Re: [Tinycc-devel] Small patch, grischka, 2013/01/31
- Re: [Tinycc-devel] Small patch, Vincent Lefevre, 2013/01/31
- Re: [Tinycc-devel] Small patch, Michael Matz, 2013/01/31
Re: [Tinycc-devel] Small patch, Thomas Preud'homme, 2013/01/31
Re: [Tinycc-devel] Small patch, Domingo Alvarez Duarte, 2013/01/31