<html>
<head>
<style><!--
.hmmessage P
{
margin:0px;
padding:0px
}
body.hmmessage
{
font-size: 12pt;
font-family:Calibri
}
--></style></head>
<body class='hmmessage'><div dir='ltr'>Thanks Hitoshi,<br><br>It is a GNU C extension, and implicitly add strerror (errno) .<br><br>Since errno is used for system errors, should we modify it or<br>we can solve the issue by just adjusting the order of sd_err and err_to_stderr?<br>Just some thoughts, <br><br>Roy <br><br><div>> Date: Wed, 4 Mar 2015 23:06:25 +0900<br>> From: mitake.hitoshi@gmail.com<br>> To: bsdnet@outlook.com<br>> CC: namei.unix@gmail.com; skypexu@gmail.com; sheepdog@lists.wpkg.org<br>> Subject: Re: [sheepdog] Unreliable error code handling in sheep/plainstore.c<br>> <br>> At Wed, 4 Mar 2015 01:14:29 -0800,<br>> Roy Yang wrote:<br>> > <br>> > [1 <multipart/alternative (7bit)>]<br>> > [1.1 <text/plain; iso-8859-1 (quoted-printable)>]<br>> > <br>> > <br>> > <br>> > Maybe I miss something here.<br>> > sd_err("failed to write object %"PRIx64", path=%s, <br>> > offset=%" PRId32 ", size=%"PRId32", result=%zd, %m", oid, path,<br>> > iocb->offset, iocb->length, size);<br>> > <br>> > 6 format positions with %, but only 5 values.<br>> > This looks a bug, and maybe the root cause why sd_err set errno?<br>> > <br>> > Reset errno inside log_write will hide such errors if this is a <br>> > real bug.<br>> <br>> The last %m doesn't require its value. It just used for print string<br>> representation of current errno.<br>> http://www.gnu.org/software/libc/manual/html_node/Other-Output-Conversions.html<br>> <br>> Thanks,<br>> Hitoshi<br>> <br>> > Roy<br>> > <br>> > > Date: Wed, 4 Mar 2015 16:31:52 +0800<br>> > > From: namei.unix@gmail.com<br>> > > To: skypexu@gmail.com<br>> > > CC: sheepdog@lists.wpkg.org<br>> > > Subject: Re: [sheepdog] Unreliable error code handling in sheep/plainstore.c<br>> > > <br>> > > On Wed, Mar 04, 2015 at 04:21:46PM +0800, Xu Yifeng wrote:<br>> > > > I found some unreliable error handling in plainstore.c, reason is<br>> > > > combination of<br>> > > > sd_err() and err_to_sderr(), because it is not guaranteed that<br>> > > > sd_err() does not<br>> > > > muck errno, it is possible passing an mucked errno to<br>> > > > err_to_sderr(), see following<br>> > > > code in plain_store.c, line 758:<br>> > > > if (unlikely(size != len)) {<br>> > > > sd_err("failed to write object %"PRIx64", path=%s,<br>> > > > offset=%"<br>> > > > PRId32", size=%"PRId32", result=%zd, %m", oid, path,<br>> > > > iocb->offset, iocb->length, size);<br>> > > > ret = err_to_sderr(path, oid, errno);<br>> > > > goto out;<br>> > > > }<br>> > > > <br>> > > > it is possible that sd_err() may muck the errno, causing an<br>> > > > irrelevant errno to be passed to<br>> > > > err_to_sderr!<br>> > > <br>> > > Good catch!<br>> > > <br>> > > It is really a hidden bug for a long time. This might explain why I sometimes saw<br>> > > strange logs in the past!<br>> > > <br>> > > Could you please cook a decent patch for this problem? The fix looks good to me.<br>> > > <br>> > > Thanks,<br>> > > Yuan<br>> > > -- <br>> > > sheepdog mailing list<br>> > > sheepdog@lists.wpkg.org<br>> > > https://lists.wpkg.org/mailman/listinfo/sheepdog<br>> > <br>> > <br>> > [1.2 <text/html; iso-8859-1 (quoted-printable)>]<br>> > <br>> > [2 <text/plain; us-ascii (7bit)>]<br>> > -- <br>> > sheepdog mailing list<br>> > sheepdog@lists.wpkg.org<br>> > https://lists.wpkg.org/mailman/listinfo/sheepdog<br></div> </div></body>
</html>