[PATCH v5 01/27] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1 }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.1.583.g721a9e0

[PATCH v4 01/25] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1 }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.1.513.g00ef6dd

Re: [PATCH v3 01/25] sequencer: use static initializers for replay_opts

[Junio C Hamano]

Johannes Schindelin  writes:

> This change is not completely faithful: instead of initializing all fields
> to 0, we choose to initialize command and subcommand to -1 (instead of
> defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
> it makes no difference at all, but future-proofs the code to require
> explicit assignments for both fields.

The assignments to opts.action immediately following, I would say
this is quite faithful conversion that looks good.

>  int cmd_revert(int argc, const char **argv, const char *prefix)
>  {
> - struct replay_opts opts;
> + struct replay_opts opts = REPLAY_OPTS_INIT;
>   int res;
>  
> - memset(, 0, sizeof(opts));
>   if (isatty(0))
>   opts.edit = 1;
>   opts.action = REPLAY_REVERT;
> @@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
> *prefix)
>  
>  int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
>  {
> - struct replay_opts opts;
> + struct replay_opts opts = REPLAY_OPTS_INIT;
>   int res;
>  
> - memset(, 0, sizeof(opts));
>   opts.action = REPLAY_PICK;
>   git_config(git_default_config, NULL);
>   parse_args(argc, argv, );
> diff --git a/sequencer.h b/sequencer.h
> index 5ed5cb1..db425ad 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -47,6 +47,7 @@ struct replay_opts {
>   /* Only used by REPLAY_NONE */
>   struct rev_info *revs;
>  };
> +#define REPLAY_OPTS_INIT { -1, -1 }
>  
>  int sequencer_pick_revisions(struct replay_opts *opts);

[PATCH v3 01/25] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1 }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.0.windows.1.325.ge6089c1

Re: [PATCH v2 01/25] sequencer: use static initializers for replay_opts

[Junio C Hamano]

Johannes Schindelin  writes:

> This change is not completely faithful: instead of initializing all fields
> to 0, we choose to initialize command and subcommand to -1 (instead of
> defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
> it makes no difference at all, but future-proofs the code to require
> explicit assignments for both fields.
>
> Signed-off-by: Johannes Schindelin 
> ---

OK.

[PATCH v2 01/25] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1 }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.0.windows.1.10.g803177d

Re: [PATCH 01/22] sequencer: use static initializers for replay_opts

[Jakub Narębski]

W dniu 29.08.2016 o 11:19, Dennis Kaarsemaker pisze:
> On ma, 2016-08-29 at 10:03 +0200, Johannes Schindelin wrote:
> 
>> +#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, 
>> NULL, NULL, 0, 0, NULL }
> 
> This looked off to me, as it replaces memset(..., 0, ...) so is not
> 100% equivalent. But the changed functions both set opts.action and
> call parse_args which sets opts.subcommand.

This information would be nice to have in the commit message.

-- 
Jakub Narębski

Re: [PATCH 01/22] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

Hi Dennis,

On Mon, 29 Aug 2016, Dennis Kaarsemaker wrote:

> On ma, 2016-08-29 at 10:03 +0200, Johannes Schindelin wrote:
> 
> > +#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, 
> > NULL, NULL, 0, 0, NULL }
> 
> This looked off to me, as it replaces memset(..., 0, ...) so is not
> 100% equivalent. But the changed functions both set opts.action and
> call parse_args which sets opts.subcommand.

Okay... Do you want me to change anything?

Ciao,
Dscho

Re: [PATCH 01/22] sequencer: use static initializers for replay_opts

[Dennis Kaarsemaker]

On ma, 2016-08-29 at 10:03 +0200, Johannes Schindelin wrote:

> +#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, 
> NULL, 0, 0, NULL }

This looked off to me, as it replaces memset(..., 0, ...) so is not
100% equivalent. But the changed functions both set opts.action and
call parse_args which sets opts.subcommand.

D.

[PATCH 01/22] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..2ca096b 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, 
NULL, 0, 0, NULL }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.0.rc1.114.g2bd6b38

static initializers

[Stefan Zager]

Howdy,

Is there any policy on making static initializers thread-safe?  I'm
working on an experimental patch to introduce threading, but I'm
running into a few non-thread-safe bits like this, in convert.c:

static const char *conv_attr_name[] = {
crlf, ident, filter, eol, text,
};
#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name)

static void convert_attrs(struct conv_attrs *ca, const char *path)
{
int i;
static struct git_attr_check ccheck[NUM_CONV_ATTRS];

if (!ccheck[0].attr) {
for (i = 0; i  NUM_CONV_ATTRS; i++)
ccheck[i].attr = git_attr(conv_attr_name[i]);
user_convert_tail = user_convert;
git_config(read_convert_config, NULL);
}
}



The easy fix would be to stick the initialization bit into an 'extern
int init_convert_config();' function, and invoke it prior to the
threaded part of my code.  That would be no worse than the current
state of affairs, which is to say that adding threading is rife with
hidden perils.

A more comprehensive fix might be:

#include pthread.h

static pthread_mutex_t convert_config_mutex = PTHREAD_MUTEX_INITIALIZER;

static void convert_attrs(struct conv_attrs *ca, const char *path)
{
int i;
static struct git_attr_check ccheck[NUM_CONV_ATTRS];

pthread_mutex_lock(convert_config_mutex);
if (!ccheck[0].attr) {
for (i = 0; i  NUM_CONV_ATTRS; i++)
ccheck[i].attr = git_attr(conv_attr_name[i]);
user_convert_tail = user_convert;
git_config(read_convert_config, NULL);
}
pthread_mutex_unlock(convert_config_mutex);
}


Unfortunately, I don't think mingw/msys supports
PTHREAD_MUTEX_INITIALIZER.  A possible workaround would be:

static pthread_mutex_t convert_config_mutex;

static void init_convert_config_mutex() __attribute__((constructor));
static void init_convert_config_mutex()
{
pthread_mutex_init(convert_config_mutex);
}


But then, I'm not whether mingw/msys supports __attribute__(constructor).


Can anyone give me some guidance before I go to much further into the
weeds (and I'm neck-deep as it is)?

Thanks,

Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] Fwd: static initializers

[Erik Faye-Lund]

On Mon, Jan 6, 2014 at 11:05 PM, Stefan Zager sza...@google.com wrote:
 Forwarding to msysgit for any guidance about win equivalents for
 PTHREAD_MUTEX_INITIALIZER or __attribute__((constructor)).

As you've probably already guessed, PTHREAD_MUTEX_INITIALIZER isn't
supported in our pthreads-emulator. I did send out a patch adding
support for it a while ago, but it hasn't been heavily tested.

__attribute__((constructor)) doesn't work on MSVC, which we also build with.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

IB/ehca: Lock renaming, static initializers

[Linux Kernel Mailing List]

Gitweb: 
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9844b71baa60270110eabaa9589d3260443d1a71
Commit: 9844b71baa60270110eabaa9589d3260443d1a71
Parent: 15f001ec47b049051f679f8b8c965ac9aae95b3e
Author: Joachim Fenkes [EMAIL PROTECTED]
AuthorDate: Mon Jul 9 15:29:03 2007 +0200
Committer:  Roland Dreier [EMAIL PROTECTED]
CommitDate: Mon Jul 9 20:12:27 2007 -0700

IB/ehca: Lock renaming, static initializers

- Rename all spinlock flags to flags, matching the vast majority of kernel
  code.
- Move hcall_lock into the only file it's used in.
- Replaced spin_lock_init() and friends with static initializers for
  global variables.

Signed-off-by: Joachim Fenkes [EMAIL PROTECTED]
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
---
 drivers/infiniband/hw/ehca/ehca_classes.h |1 -
 drivers/infiniband/hw/ehca/ehca_cq.c  |   12 ++--
 drivers/infiniband/hw/ehca/ehca_main.c|   18 --
 drivers/infiniband/hw/ehca/ehca_qp.c  |6 +++---
 drivers/infiniband/hw/ehca/ehca_reqs.c|   24 
 drivers/infiniband/hw/ehca/hcp_if.c   |2 ++
 6 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_classes.h 
b/drivers/infiniband/hw/ehca/ehca_classes.h
index 9d689ae..3550047 100644
--- a/drivers/infiniband/hw/ehca/ehca_classes.h
+++ b/drivers/infiniband/hw/ehca/ehca_classes.h
@@ -295,7 +295,6 @@ void ehca_cleanup_mrmw_cache(void);
 
 extern spinlock_t ehca_qp_idr_lock;
 extern spinlock_t ehca_cq_idr_lock;
-extern spinlock_t hcall_lock;
 extern struct idr ehca_qp_idr;
 extern struct idr ehca_cq_idr;
 
diff --git a/drivers/infiniband/hw/ehca/ehca_cq.c 
b/drivers/infiniband/hw/ehca/ehca_cq.c
index 67f0670..94bad27 100644
--- a/drivers/infiniband/hw/ehca/ehca_cq.c
+++ b/drivers/infiniband/hw/ehca/ehca_cq.c
@@ -56,11 +56,11 @@ int ehca_cq_assign_qp(struct ehca_cq *cq, struct ehca_qp 
*qp)
 {
unsigned int qp_num = qp-real_qp_num;
unsigned int key = qp_num  (QP_HASHTAB_LEN-1);
-   unsigned long spl_flags;
+   unsigned long flags;
 
-   spin_lock_irqsave(cq-spinlock, spl_flags);
+   spin_lock_irqsave(cq-spinlock, flags);
hlist_add_head(qp-list_entries, cq-qp_hashtab[key]);
-   spin_unlock_irqrestore(cq-spinlock, spl_flags);
+   spin_unlock_irqrestore(cq-spinlock, flags);
 
ehca_dbg(cq-ib_cq.device, cq_num=%x real_qp_num=%x,
 cq-cq_number, qp_num);
@@ -74,9 +74,9 @@ int ehca_cq_unassign_qp(struct ehca_cq *cq, unsigned int 
real_qp_num)
unsigned int key = real_qp_num  (QP_HASHTAB_LEN-1);
struct hlist_node *iter;
struct ehca_qp *qp;
-   unsigned long spl_flags;
+   unsigned long flags;
 
-   spin_lock_irqsave(cq-spinlock, spl_flags);
+   spin_lock_irqsave(cq-spinlock, flags);
hlist_for_each(iter, cq-qp_hashtab[key]) {
qp = hlist_entry(iter, struct ehca_qp, list_entries);
if (qp-real_qp_num == real_qp_num) {
@@ -88,7 +88,7 @@ int ehca_cq_unassign_qp(struct ehca_cq *cq, unsigned int 
real_qp_num)
break;
}
}
-   spin_unlock_irqrestore(cq-spinlock, spl_flags);
+   spin_unlock_irqrestore(cq-spinlock, flags);
if (ret)
ehca_err(cq-ib_cq.device,
 qp not found cq_num=%x real_qp_num=%x,
diff --git a/drivers/infiniband/hw/ehca/ehca_main.c 
b/drivers/infiniband/hw/ehca/ehca_main.c
index ca215ba..32396b2 100644
--- a/drivers/infiniband/hw/ehca/ehca_main.c
+++ b/drivers/infiniband/hw/ehca/ehca_main.c
@@ -96,15 +96,13 @@ MODULE_PARM_DESC(static_rate,
 MODULE_PARM_DESC(scaling_code,
 set scaling code (0: disabled/default, 1: enabled));
 
-spinlock_t ehca_qp_idr_lock;
-spinlock_t ehca_cq_idr_lock;
-spinlock_t hcall_lock;
+DEFINE_SPINLOCK(ehca_qp_idr_lock);
+DEFINE_SPINLOCK(ehca_cq_idr_lock);
 DEFINE_IDR(ehca_qp_idr);
 DEFINE_IDR(ehca_cq_idr);
 
-
-static struct list_head shca_list; /* list of all registered ehcas */
-static spinlock_t shca_list_lock;
+static LIST_HEAD(shca_list); /* list of all registered ehcas */
+static DEFINE_SPINLOCK(shca_list_lock);
 
 static struct timer_list poll_eqs_timer;
 
@@ -864,14 +862,6 @@ int __init ehca_module_init(void)
 
printk(KERN_INFO eHCA Infiniband Device Driver 
   (Rel.: SVNEHCA_0023)\n);
-   idr_init(ehca_qp_idr);
-   idr_init(ehca_cq_idr);
-   spin_lock_init(ehca_qp_idr_lock);
-   spin_lock_init(ehca_cq_idr_lock);
-   spin_lock_init(hcall_lock);
-
-   INIT_LIST_HEAD(shca_list);
-   spin_lock_init(shca_list_lock);
 
if ((ret = ehca_create_comp_pool())) {
ehca_gen_err(Cannot create comp pool.);
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c 
b/drivers/infiniband/hw/ehca/ehca_qp.c
index ec34ff4..31d2152 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw