[PATCH 1/2] ar: add archive creation

[Alexander Shishkin]

On Mon, Mar 15, 2010 at 03:39:48 +0100, Denys Vlasenko wrote:
> > +lib-$(CONFIG_FEATURE_AR_CREATE)        += ar_create.o
> 
> why this is in the separate .c file? it can be in ar.c

No reason.

> > +#if ENABLE_FEATURE_AR_CREATE
> > +       if (opt & (AR_OPT_CREATE | AR_OPT_INSERT))
> > +               return write_ar_archive(archive_handle, opt & AR_OPT_INSERT);
> > +#endif
> 
> After a bit of analysis it looks like AR_OPT_CREATE is a no-op.
> It has no effect w/o AR_OPT_INSERT, and does not affect anything
> if AR_OPT_INSERT is specified. Why we test it here then?

Yes, it seems to be a noop generally.

> 
> 
> > +       /* if uid/gid is greater than fits in ar header, crop to zero */
> > +       if (fh->uid > AR_GUID_MAX)
> > +               fh->uid = 0;
> 
> defines like AR_GUID_MAX are useless. It's harder to check
> their correctness than if they were plain 99999 constants here.
> 
> > +       /* GNU ar will create a malformed archive in this case */
> > +       if (fh->size > AR_SIZE_MAX) {
> > +               fh->size = AR_SIZE_MAX;
> > +               bb_error_msg("%s is bigger than ar can handle, croping to "
> > +                            "%"OFF_FMT"u\n", fh->name, AR_SIZE_MAX);
> > +       }
> > +
> > +       fdprintf(handle->src_fd, "%-16s%-12lu%-6u%-6u%-8o%-10"OFF_FMT"u`\n",
> > +                               fh->name, (long)fh->mtime,
> > +                               (int)fh->uid, (int)fh->gid,
> > +                               (int)fh->mode, fh->size
> 
> I tested this code and it does not work - neither we nor GNU ar
> support anything bigger than UINT_MAX (for us, the limit is on
> unpack side, where we have another buglet limiting size to ~1G).

True and I'm not sure if it's really worth fixing.

> 
> > +       char fn_h[FILENAME_MAX];
> 
> multi-kilobyte buffer in order to store 17 bytes.
> resulting code bloat: ~45 bytes.

That was because I thought I'd implement long file names right away, but ran
out of steam sooner. :)

> 
> 
> > +#if ENABLE_FEATURE_AR_LONG_FILENAMES
> > +#error nope, not yet
> > +#else
> > +       sprintf(fn_h, "%.15s/", bb_basename(fn));
> > +#endif
> 
> This breaks randomconfig tests when they pick FEATURE_AR_LONG_FILENAMES=y.
> 
> 
> Fixed and applied to git, please test it.

Works for me. Thanks!

Regards,
--
Alex