Re: [PATCH] Fix git to be (more) ANSI C99 compliant.

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

 



Hi,

On Sun, Jun 18, 2006 at 11:07:49AM +0300, Timo Hirvonen wrote:
> Many of the void-pointer arithmetic warnings could be fixed by
> changing the variable types to char * instead of casting them in many
> places.

true. Casting to a void-pointer can be done implicitely (even according
to the standard), so changing a function's argument from `void *' to
`char *' might break code somewhere else.

> > -			struct {
> > -				struct combine_diff_path p;
> > -				struct combine_diff_parent filler[5];
> > -			} combine;
> 
> Yes this is somewhat ugly but avoids a malloc.

No, malloc is still needed to allocate `combine.p.path'. My change does
allocate all memory in one pass, so the number of calls to malloc
doesn't differ.

> Leave the code as it is now unless it causes real problems.

`struct combine_diff_path' has a flexible array member (FAM) and is
therefore an incomplete type. This means you may not instanciate it.

GCC ignores the FAM in this case and allocates `sizeof (struct
combine_diff_path)' bytes. However, this is not correct according to
ANSI and prevents building using other compilers (e.g. Sun cc).

To be honest, I don't get the point of FAMs anyways. Why don't we just
use a pointer to `struct combine_diff_parent' there in the first place?

> 'A'...'Z' is more readable.  Does some compiler fail to compile it?

It's not in ANSI, therefore GCC with `-ansi' failes, so does the Sun cc.

> >  static inline int needs_quote(int ch)
> 
> Hmm.. same function in http-fetch.c.  Lots of common code could be
> moved to http.h.

Right, I wanted to keep the changes as minimal as possible, though.

As you can probably tell my goal is building a Solaris version of git
using the Sun cc. I'm not quite there yet and don't have time to look
into the remaining issue(s) today nor tomorrow, but I might have some
time at the end of the week.

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]