git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Jeff Hostetler <git@jeffhostetler.com>,
	Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 0/1] trace2: fix tracing when NO_PTHREADS is defined
Date: Tue, 28 May 2019 02:28:05 -0400	[thread overview]
Message-ID: <20190528062804.GE7946@sigill.intra.peff.net> (raw)
In-Reply-To: <CACsJy8D7w4sC_tchx-Q80PCiu+2hYfkZo22_Vb3vgJ+xvgUAAA@mail.gmail.com>

On Sat, May 25, 2019 at 05:43:55PM +0700, Duy Nguyen wrote:

> > +typedef struct {
> > +       void *data;
> > +       /* extra indirection because setspecific is passed key by value */
> > +       void **vdata;
> 
> Ha! I was thinking a separate key->value mapping which is complicated
> in C. But this works pretty well for a single thread, and it even
> supports multiple keys.

I really wish that all of the functions passed the pthread_key_t by
reference. That would make it possible to define the key as a single
pointer.

I'm not sure if pthread_key_t's are meant to be shallow-copyable. I.e.,
should this work:

  void foo(pthread_key_t *out)
  {
	pthread_key_t tmp;
	pthread_key_create(&tmp, NULL);
	*out = tmp;
  }
  ...
  pthread_key_t k;
  foo(&k);
  pthread_setspecific(k, some_ptr);

It does not with my proposed plan, because the pointer in tmp.data went
out of scope, leaving tmp.vdata (and thus k.vdata) as a dangling
pointer.

The code above seems like a vaguely crazy thing to do. But if we want to
be absolutely paranoid, we'd have to malloc() an extra pointer in the
create() function, instead of carrying it inside the key. Or just make a
global "void *thread_specific_data[PTHREAD_KEYS_MAX]" and make each key
an integer index into it.

It's pretty clear that they expect one of those two implementations,
given that POSIX says key creation can report either ENOMEM, or EAGAIN
if we exceed PTHREAD_KEYS_MAX. :)

-Peff

  reply	other threads:[~2019-05-28  6:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-21 19:33 Jeff Hostetler via GitGitGadget
2019-05-21 19:33 ` [PATCH 1/1] " Jeff Hostetler via GitGitGadget
2019-05-21 21:27 ` [PATCH 0/1] " Jeff King
2019-05-22 13:23   ` Jeff Hostetler
2019-05-23  5:51     ` Jeff King
2019-05-23 13:55       ` Jeff Hostetler
2019-05-25 10:43       ` Duy Nguyen
2019-05-28  6:28         ` Jeff King [this message]
2019-05-28 14:57           ` Jeff Hostetler
2019-05-28 17:14     ` Junio C Hamano

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190528062804.GE7946@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=pclouds@gmail.com \
    --subject='Re: [PATCH 0/1] trace2: fix tracing when NO_PTHREADS is defined' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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).