This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][LTO][RFC] AR archive support for LTO


> > I don't think we can assume that ar.h is available, so I'd suggest
> > to hardcode 60 here with an appropriate comment.

> Done. Updated patch attached.

In lto-plugin.c:

+      Elf *archive;
+      off_t offset = file->offset - sizeof(struct ar_hdr);
+      int t = asprintf (&objname, "%s@%" PRId64, file->name, file->offset);

Is it still OK to include <ar.h> from lto-plugin.c? I'm not sure why
it was necessary to remove it from lto-elf.c, but it's OK for
lto-plugin.c.

I don't think using PRId64 with off_t is completely portable. I'd
suggest casting file->offset to (int64_t) here.

Also, I think a comment here is necessary to make it clear that you
really do mean to pass "file->offset" as part of the file name rather
than the adjusted "offset" computed on the line above -- i.e., that
lto will make the same adjustment when parsing the filename. (Or
perhaps move the computation of "offset" down further to where it's
actually used.)

+      check (elf_kind(archive) == ELF_K_AR, LDPL_FATAL,
+             "Not an archive and offset not 0");
+
+      check (offset == elf_rand(archive, offset), LDPL_FATAL,
+             "could not seek in archive");
+      elf = elf_begin(file->fd, ELF_C_READ, archive);

Need spaces after elf_kind, elf_rand, and elf_begin.

In lto-elf.c:

+  off_t offset;
...
+      sscanf(offset_p, "%" PRId64, &offset);

Same problem as above; I don't think there's a portable format
specifier for off_t. I'd suggest declaring offset as int64_t here and
casting to off_t when you need it, or assigning it to an off_t after
the sscanf.

+      fname = (char *)xmalloc (offset_p - filename + 1);

Space after the cast?

+      off_t t = elf_rand(elf_file->elf, offset);
+      if (t != offset)
+        {
+          error ("could not seek in archive");
+          goto fail;
+        }
+
+      e = elf_begin(elf_file->fd, ELF_C_READ, elf_file->elf);

Need spaces after elf_rand and elf_begin.

OK with those changes.

-cary


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]