On 6/30/10 2:35 PM, Carl-Daniel Hailfinger wrote: > Using the superiotool code to detect SuperI/O chips can be really useful > even for applications besides superiotool, e.g. flashrom. > The biggest hurdle right now is the excessive size of the superiotool > binary (>2.2 MB) which is still ~15 kB after lzma compression. That's > simply unacceptable for a library. > Why? There is no size constraints on libraries in Unix like systems.
Assuming we're talking about a static library >From my system: 20M /usr/lib64/libbfd.a 20M /usr/lib64/libc.a 11M /usr/lib64/libcrypto.a 19M /usr/lib64/libdb-4.5.a 3.9M /usr/lib64/libfreetype.a 5.1M /usr/lib64/libgio-2.0.a 3.6M /usr/lib64/libglib-2.0.a 3.3M /usr/lib64/libgmp.a 2.8M /usr/lib64/libm.a 2.8M /usr/lib64/libncurses.a 3.2M /usr/lib64/libncursesw.a 4.9M /usr/lib64/libopcodes.a 4.2M /usr/lib64/libruby-static.a 3.0M /usr/lib64/libssl.a 6.9M /usr/lib64/libxml2.a Also, keep in mind that the size of the library in the filesystem is not the size increase when linking it. 15kb lzma compressed is nothing. > This minimally invasive patch makes all dumping code optional and > reduces binary size for the probe-only case (libsuperiodetect) a lot. > Binary sizes: > 2268258 (before) > 31600 (after) 98.6% reduction > LZMA compressed and stripped binary sizes: > 14778 (before) > 6709 (after) 54.6% reduction > Not worth the additional unreadability of the code in my opinion > To test compilation of superiotool with probe-only functionality, run > make CONFIG_LIB=yes > > This patch is the first step towards libsuperiodetect, and more will > have to follow. > I think the idea for libsuperio is great. I'll gladly ack this without the very intrusive "we save 6700 bytes" part of the patch. > IMHO the {EOT} markers should be killed in libsuperiodetect, at least > for the LDNs. Such an operation would have increased the size of the > patch and would have reduced readability as well. Due to that, I skipped > it, but I plan to do that in a later operation. > What would you use instead? The array members all have different sizes as far as I can tell, so some kind of end marker will be needed. > Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2...@gmx.net> > > /* Init: 0x55, 0x55. Exit: 0xaa. Port: 0x3f0. */ > @@ -765,7 +803,9 @@ > uint8_t revreg) > { > uint8_t id, rev; > +#ifndef LIBSUPERIODETECT > uint16_t runtime_base; > +#endif /* ! LIBSUPERIODETECT */ > Maybe this could be moved downwards to where it is used, to save an ifndef? > Index: superiotool_libsuperiodetect/superiotool.h > =================================================================== > --- superiotool_libsuperiodetect/superiotool.h (Revision 5651) > +++ superiotool_libsuperiodetect/superiotool.h (Arbeitskopie) > @@ -98,12 +98,19 @@ > struct superio_registers { > int32_t superio_id; /* Signed, as we need EOT. */ > const char *name; /* Super I/O name */ > +#ifndef LIBSUPERIODETECT > struct { > int8_t ldn; > const char *name; /* LDN name */ > int16_t idx[IDXSIZE]; > int16_t def[IDXSIZE]; > } ldn[LDNSIZE]; > +#else /* LIBSUPERIODETECT */ > + /* Ugly hack to avoid messing with EOT markers. */ > + struct { > + int8_t dummy; > + } dummy[1]; > +#endif /* LIBSUPERIODETECT */ > }; > hm ... the comment is very terse. -- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: i...@coresystems.de • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot