unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Florian Weimer (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	libc-alpha@sourceware.org
Cc: Florian Weimer <fweimer@redhat.com>
Subject: [review] nptl: Cleanup mutex internal offset tests
Date: Thu, 14 Nov 2019 09:34:51 -0500	[thread overview]
Message-ID: <20191114143451.94BF920AF6@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1573242232000.I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f@gnutoolchain-gerrit.osci.io>

Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/581
......................................................................


Patch Set 1:

(5 comments)

I ran this through a full build-many-glibcs.py run, and the results look good.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,18 @@ 
| +Parent:     31f000a8 (Remove hppa pthreadP.h)
| +Author:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +AuthorDate: 2019-11-07 20:58:41 +0000
| +Commit:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +CommitDate: 2019-11-08 16:41:53 -0300
| +
| +nptl: Cleanup mutex internal offset tests
| +
| +The offset of pthread_mutex_t __data.__nusers, __data.__spins,

PS1, Line 9:

“offsets” (I think)

| +__data.elision, __data.list are not required to be constant over
| +the releases.  Only the __data.__flags are used for static
| +initializers.

PS1, Line 12:

“Only __data.__kind is used for static initializers.”

| +
| +This patch also adds an additional size check for __data.__flags.

PS1, Line 14:

__data.__kind

| +
| +Check with a build against affected abis.

PS1, Line 16:

“Checked with a build against affected ABIs.”?

| +
| +Change-Id: I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f
| --- nptl/pthread_mutex_init.c
| +++ nptl/pthread_mutex_init.c
| @@ -51,17 +51,11 @@ int
|  __pthread_mutex_init (pthread_mutex_t *mutex,
|  		      const pthread_mutexattr_t *mutexattr)
|  {
|    const struct pthread_mutexattr *imutexattr;
|  
|    ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
|  
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
| -				  __PTHREAD_MUTEX_NUSERS_OFFSET);
| +  /* The __flags is the only field where its offset should be checked to

PS1, Line 58:

Probably: /* __kind is the only field
(without the definite article, and fixed field name).

| +     avoid ABI breakage with static initializers.  */
|    ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__kind,
|  				  __PTHREAD_MUTEX_KIND_OFFSET);
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__spins,
| -				  __PTHREAD_MUTEX_SPINS_OFFSET);
| -#if __PTHREAD_MUTEX_LOCK_ELISION
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__elision,
| -				  __PTHREAD_MUTEX_ELISION_OFFSET);
| -#endif

-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f
Gerrit-Change-Number: 581
Gerrit-PatchSet: 1
Gerrit-Owner: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Gerrit-CC: Florian Weimer <fweimer@redhat.com>
Gerrit-Comment-Date: Thu, 14 Nov 2019 14:34:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

  reply	other threads:[~2019-11-14 14:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08 19:43 [review] nptl: Cleanup mutex internal offset tests Adhemerval Zanella (Code Review)
2019-11-14 14:34 ` Florian Weimer (Code Review) [this message]
2019-11-20 14:49 ` Adhemerval Zanella (Code Review)
2019-11-20 14:51 ` [review v2] " Adhemerval Zanella (Code Review)
2019-11-22 11:09 ` Florian Weimer (Code Review)
2019-11-26 14:20 ` [pushed] " Sourceware to Gerrit sync (Code Review)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191114143451.94BF920AF6@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=adhemerval.zanella@linaro.org \
    --cc=fweimer@redhat.com \
    --cc=gnutoolchain-gerrit@osci.io \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).