[sheepdog] Unreliable error code handling in sheep/plainstore.c

[Hitoshi Mitake]

At Wed, 4 Mar 2015 01:14:29 -0800,
Roy Yang wrote:
> 
> [1  <multipart/alternative (7bit)>]
> [1.1  <text/plain; iso-8859-1 (quoted-printable)>]
> 
> 
> 
> Maybe I miss something here.
> sd_err("failed to write object %"PRIx64", path=%s, 
> offset=%" PRId32 ", size=%"PRId32", result=%zd, %m", oid, path,
>                         iocb->offset, iocb->length, size);
> 
> 6 format positions with %, but only 5 values.
> This looks a bug, and maybe the root cause why sd_err set errno?
> 
> Reset errno inside log_write will hide such errors if this is a 
> real bug.

The last %m doesn't require its value. It just used for print string
representation of current errno.
http://www.gnu.org/software/libc/manual/html_node/Other-Output-Conversions.html

Thanks,
Hitoshi

> Roy
> 
> > Date: Wed, 4 Mar 2015 16:31:52 +0800
> > From: namei.unix at gmail.com
> > To: skypexu at gmail.com
> > CC: sheepdog at lists.wpkg.org
> > Subject: Re: [sheepdog] Unreliable error code handling in sheep/plainstore.c
> > 
> > On Wed, Mar 04, 2015 at 04:21:46PM +0800, Xu Yifeng wrote:
> > > I found some unreliable error handling in plainstore.c, reason is
> > > combination of
> > > sd_err() and err_to_sderr(), because it is not guaranteed that
> > > sd_err() does not
> > > muck errno, it is possible passing an mucked errno to
> > > err_to_sderr(), see following
> > > code in plain_store.c, line 758:
> > >    if (unlikely(size != len)) {
> > >                 sd_err("failed to write object %"PRIx64", path=%s,
> > > offset=%"
> > >                        PRId32", size=%"PRId32", result=%zd, %m", oid, path,
> > >                        iocb->offset, iocb->length, size);
> > >                 ret = err_to_sderr(path, oid, errno);
> > >                 goto out;
> > >         }
> > > 
> > > it is possible that sd_err() may muck the errno, causing an
> > > irrelevant errno to be passed to
> > > err_to_sderr!
> > 
> > Good catch!
> > 
> > It is really a hidden bug for a long time. This might explain why I sometimes saw
> > strange logs in the past!
> > 
> > Could you please cook a decent patch for this problem? The fix looks good to me.
> > 
> > Thanks,
> > Yuan
> > -- 
> > sheepdog mailing list
> > sheepdog at lists.wpkg.org
> > https://lists.wpkg.org/mailman/listinfo/sheepdog
> 
>  		 	   		  
> [1.2  <text/html; iso-8859-1 (quoted-printable)>]
> 
> [2  <text/plain; us-ascii (7bit)>]
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> https://lists.wpkg.org/mailman/listinfo/sheepdog