Re: [PATCH 01/18] Introduce fsck options

2014-12-22 Thread Johannes Schindelin 
Hi Junio,

On Wed, 10 Dec 2014, Junio C Hamano wrote:

 Johannes Schindelin johannes.schinde...@gmx.de writes:
 
  Subject: Re: [PATCH 01/18] Introduce fsck options
 
 please make it easier to cluster and spot the series in the eventual
 shortlog by giving a common prefix to the patches, e.g.
 
   fsck: introduce fsck_options struct

I use the fsck: prefix consistently now.

  +static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
  +static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
 
 Is it a good idea to allow walker to be strict but obj verifier to
 be not (or vice versa)?  I am wondering why this is not a single
 struct with two callback function pointers.

Unfortunately not. There are two different walkers used, and in fact,
fsck_walk_options() is only used to walk the objects, not to fsck them.

Now, I could use only one struct and set the walker, but that is not
thread-safe, and while code is not threaded yet AFAICT, it might be in the
future. That is why I decided to be rather safe than sorry. If you want it
differently, please just say the word, I will make it so.

  +struct fsck_options {
  +   fsck_walk_func walk;
  +   fsck_error error_func;
  +   int strict:1;
 
 A signed 1-bit-wide bitfield can hold its sign-bit and nothing else,
 no?
 
 unsigned strict:1;

Oops. Right. For some reason, it worked here, though... Fixed!

Ciao,
Dscho
--
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: [PATCH 01/18] Introduce fsck options

2014-12-22 Thread Junio C Hamano 
Johannes Schindelin johannes.schinde...@gmx.de writes:

 Is it a good idea to allow walker to be strict but obj verifier to
 be not (or vice versa)?  I am wondering why this is not a single
 struct with two callback function pointers.

 Unfortunately not. There are two different walkers used, and in fact,
 fsck_walk_options() is only used to walk the objects, not to fsck them.

 Now, I could use only one struct and set the walker, but that is not
 thread-safe, and while code is not threaded yet AFAICT, it might be in the
 future. That is why I decided to be rather safe than sorry. If you want it
 differently, please just say the word, I will make it so.

Thanks for explaining; I just found that the reason behind the
design choice was unclear and wanted to know.
--
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: [PATCH 01/18] Introduce fsck options

2014-12-10 Thread Junio C Hamano 
Johannes Schindelin johannes.schinde...@gmx.de writes:

THis is not limited to this step, but

 Subject: Re: [PATCH 01/18] Introduce fsck options

please make it easier to cluster and spot the series in the eventual
shortlog by giving a common prefix to the patches, e.g.

fsck: introduce fsck_options struct

 +static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
 +static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;

Is it a good idea to allow walker to be strict but obj verifier to
be not (or vice versa)?  I am wondering why this is not a single
struct with two callback function pointers.

 +struct fsck_options {
 + fsck_walk_func walk;
 + fsck_error error_func;
 + int strict:1;

A signed 1-bit-wide bitfield can hold its sign-bit and nothing else,
no?

unsigned strict:1;

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