[sheepdog] [PATCH] lib: choose a correct output in logger

[Liu Yuan]

On Thu, Jul 18, 2013 at 11:13:44AM +0900, Hitoshi Mitake wrote:
> At Thu, 18 Jul 2013 00:38:33 +0900,
> Hitoshi Mitake wrote:
> > 
> > At Wed, 17 Jul 2013 16:51:57 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Wed, Jul 17, 2013 at 04:48:10PM +0900, Hitoshi Mitake wrote:
> > > > At Tue, 16 Jul 2013 15:01:07 +0800,
> > > > Kai Zhang wrote:
> > > > > 
> > > > > 
> > > > > On Jul 15, 2013, at 9:03 PM, Hitoshi Mitake <mitake.hitoshi at gmail.com> wrote:
> > > > > 
> > > > > > Current logger chooses stderr as an output of logs in cases
> > > > > > logarea_init() isn't called or default formatter is used.
> > > > > > 
> > > > > > This patch lets logger choose stderr when a priority of logs is less
> > > > > > than or equal to LOG_NOTICE. If the priority is larger than or equal
> > > > > > to SDOG_INFO, stdout is used.
> > > > > > 
> > > > > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > > > > ---
> > > > > > lib/logger.c |    6 ++++--
> > > > > > 1 files changed, 4 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/lib/logger.c b/lib/logger.c
> > > > > > index 253163d..059140f 100644
> > > > > > --- a/lib/logger.c
> > > > > > +++ b/lib/logger.c
> > > > > > @@ -407,8 +407,10 @@ static notrace void dolog(int prio, const char *func, int line,
> > > > > > 		memset(msg, 0, sizeof(struct logmsg));
> > > > > > 		init_logmsg(msg, &tv, prio, func, line);
> > > > > > 		len = format->formatter(str_final, MAX_MSG_SIZE, msg);
> > > > > > -		xwrite(fileno(stderr), str_final, len);
> > > > > > -		fflush(stderr);
> > > > > > +
> > > > > > +		FILE *f = SDOG_INFO <= prio ? stdout : stderr;
> > > > > > +		xwrite(fileno(f), str_final, len);
> > > > > > +		fflush(f);
> > > > > > 	}
> > > > > > }
> > > > > > 
> > > > > 
> > > > > It is odd to declare a variable 'f' in the middle of the method.
> > > > 
> > > > No. We employed gnu99 style in 7141b4535bad, interleaving declarations
> > > > and statements are allowed.
> > > > 
> > > > Declaring variables at top of functions has no meaning. Narrowed
> > > > scope of variables improves readability significantly.
> > > > 
> > > 
> > > What do you mean by 'improves readablility significantly'? It seems that you
> > > assume we write big complex function, in which case declaration in the middle
> > > might help people get the idea what the variable is. But this assumption is
> > > fundamentally wrong to me. Trust me, big function will always be harder to
> > > understand than partitioning it into smaller well organized functions. Any
> > > effort of trying to improve the readability of a big & complex function will be
> > > in vain.
> > 
> > Improvement of readability obtained by smaller scope is not limited to
> > big, complex functions. e.g. there are many blocks which has variable
> > declarations on top of it. e.g.
> > 	for (i = start; i < end; i++) {
> > 		size_t nr_oids;
> > 		struct sd_node *node = cur + i;
> > (from prepare_object_list())
> > 
> > we write variable declarations on top of these blocks because it is a
> > efficient way to express that these variables are not used in outside
> > of the blocks.
> > 
> > Interleaving of declarations and statements is a natural expansion of
> > it. The declaration of *f in my patch expresses *f is not used in the
> > predecessor statements. Of course the block is small and its
> > readability will not be a problem. But I don't know any reason of why
> > we _shouldn't_ write the declaration on the top of the block.
> 
> Sorry, the above is my mistake. I don't know any reason of why we
> _shouldn't_ write the declaration between statements.

It looks messy to me. What benefit we get from it? Narrower scope doesn't
improve code readability.

Thanks
Yuan