Re: [Tinycc-devel] [PATCH] Add support for reading thin archive files.

[grischka]

On 30.10.2023 08:41, Reimar Döffinger wrote:
> > +        if (0 == memcmp(h, "!<thin>\n", 8))
> > +            return AFF_BINTYPE_AR;
> > "!<thin>\n" looks like a candidate for a symbolic name not unlike ARMAG ?)
>
> It seemed overkill at first since it was only used in one place,
> but now that it's used in 2 places, yes.

Hi,  well, it probably shouldn't be used in two places.  You could
return AFF_BINTYPE_AR_THIN for example to avoid the redundant extra
check.

Is it that in your code I do notice some general tendency to
duplications?  Like
            if (is_thin) size = 0;
            if (is_thin && tcc_add_ar_thin_file(s1, filename, hdr.ar_name, 
fnames, fnames_size) != 0) {
            ...
Plus 2 locations where it reads the "//" long names.
Also in
"support for codesigning command"- Seem to recall that it already did exist ?!?
"-MP option" - quite some copy & paste (and a very big comment to my taste).
"delete created ar file on error" - "created_file": 'fh' already
means the file was created and argv[i_lib] has the name.

Anyway, as we know, providing two (or more) things to do (almost) the
same is a well proven method to cause confusion.  Try to get rid of that.

Also, not necessarily was tcc written with any not yet existing
features already in mind.  If you can organize existing stuff differently
to make the new feature fit more nicely, then why not ... ;)

-- grischka

> Best regards,
> Reimar