This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: [COMMITTED] Add comments for processing extra pseudo-hwcaps from vDSOs.


On 05/16/2013 03:21 PM, Roland McGrath wrote:
>> +	    /* The standard ELF note layout is exactly as the anonymous struct.
>> +	       The next element is a variable length vendor name of length
>> +	       VENDORLEN (with a real length rounded to ElfW(Addr)), followed
>> +	       by the data of length DATALEN (with a real length rounded to
>> +	       ElfW(Addr)).  */
> 
> Actually those sizes are rounded up to ElfW(Word), which is to say, always
> four bytes.  The note header fields are four bytes even in ELFCLASS64.

Good catch. Fixed.

>> +  /* Is this a normal hwcap for the machine e.g. fpu?  */
> 
> This is a run-on sentence with no punctuation before "e.g."
> Either a comma, or parenthesizing the "e.g. clause", would be appropriate.
> (Same issue occurs twice more below.)

Fixed x 3.

>> +  /* The last entry in hwcap_extra is reserved for the "tls"
>> +     pseudo-hwcap which indicates support for TLS.  */
>>    hwcap_extra[63 - _DL_FIRST_EXTRA] = "tls";
> 
> Perhaps say here that this seemingly pointless addition is for
> compatibility with the directory layout needed for old versions,
> where "tls" was actually optional.

Fixed.

How's this?

2013-05-16  Carlos O'Donell  <carlos@redhat.com>

	* elf/dl-hwcaps.c (_dl_important_hwcaps): Comment should say round to ElfW(Word).
	* elf/ldconfig.c (is_hwcap_platform): Make comments complete sentences.
	(main): Pseudo-hwcap "tls" is legacy.

diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c
index 1b7fe52..94cbf6c 100644
--- a/elf/dl-hwcaps.c
+++ b/elf/dl-hwcaps.c
@@ -68,9 +68,9 @@ _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
                                      + GLRO(dl_sysinfo_map)->l_addr);
            /* The standard ELF note layout is exactly as the anonymous struct.
               The next element is a variable length vendor name of length
-              VENDORLEN (with a real length rounded to ElfW(Addr)), followed
+              VENDORLEN (with a real length rounded to ElfW(Word)), followed
               by the data of length DATALEN (with a real length rounded to
-              ElfW(Addr)).  */
+              ElfW(Word)).  */
            const struct
            {
              ElfW(Word) vendorlen;
diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index 340c132..9caed4a 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -173,17 +173,17 @@ is_hwcap_platform (const char *name)
 {
   int hwcap_idx = _dl_string_hwcap (name);
 
-  /* Is this a normal hwcap for the machine e.g. fpu?  */
+  /* Is this a normal hwcap for the machine like "fpu?"  */
   if (hwcap_idx != -1 && ((1 << hwcap_idx) & hwcap_mask))
     return 1;
 
-  /* ... Or is it a platform pseudo-hwcap e.g. i686?  */
+  /* Is this a platform pseudo-hwcap like "i686?"  */
   hwcap_idx = _dl_string_platform (name);
   if (hwcap_idx != -1)
     return 1;
 
-  /* ... Or is this one of the extra pseudo-hwcaps that we map beyond
-     _DL_FIRST_EXTRA e.g. tls, or nosegneg?  */
+  /* Is this one of the extra pseudo-hwcaps that we map beyond
+     _DL_FIRST_EXTRA like "tls", or "nosegneg?"  */
   for (hwcap_idx = _DL_FIRST_EXTRA; hwcap_idx < 64; ++hwcap_idx)
     if (hwcap_extra[hwcap_idx - _DL_FIRST_EXTRA] != NULL
        && !strcmp (name, hwcap_extra[hwcap_idx - _DL_FIRST_EXTRA]))
@@ -1269,8 +1269,10 @@ main (int argc, char **argv)
          add_dir (argv[i]);
     }
 
-  /* The last entry in hwcap_extra is reserved for the "tls"
-     pseudo-hwcap which indicates support for TLS.  */
+  /* The last entry in hwcap_extra is reserved for the "tls" pseudo-hwcap which
+     indicates support for TLS.  This pseudo-hwcap is only used by old versions
+     under which TLS support was optional.  The entry is no longer needed, but
+     must remain for compatibility.  */
   hwcap_extra[63 - _DL_FIRST_EXTRA] = "tls";
 
   set_hwcap ();
---

Cheers,
Carlos.


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