[prev in list] [next in list] [prev in thread] [next in thread] 

List:       subversion-dev
Subject:    Re: svn commit: r1544736 - /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
From:       Branko Čibej <brane () wandisco ! com>
Date:       2013-11-25 14:22:12
Message-ID: 52935D14.2060708 () wandisco ! com
[Download RAW message or body]

On 25.11.2013 13:05, Julian Foad wrote:
> Branko Čibej wrote:
> > > Author: stefan2
> > > URL: http://svn.apache.org/r1544736
> > > * subversion/libsvn_ra_svn/marshal.c
> > > (svn_ra_svn__handle_command): code cleanup. No functional change
> > Your change modifies the *terminate output parameter even if the
> > function fails. We have a long-standing policy against doing that.
> > You're taking too much for granted.
> We have a general policy, as I understand it, that if a function returns an error \
> then we don't promise anything about what else it has done. On that basis, this \
> change is fine. Is there some local policy overriding that general one in this \
> case?

Hm .... I remember differently. Not touching output parameters in the
case of an error is a very nice property to have in an API (it makes
debugging just that little bit easier, for example). But now I can't
find an explicit statement to this effect in HACKING. On the other hand,
we don't document that values of output parameters are indeterminate
unless the function succeeds.

Oh well. I knew senility would kick in at some point, I just didn't
expect it so soon.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 25.11.2013 13:05, Julian Foad wrote:<br>
    </div>
    <blockquote
      cite="mid:1385381152.90452.YahooMailNeo@web186105.mail.ir2.yahoo.com"
      type="cite">
      <pre wrap="">Branko Čibej wrote:
</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="">Author: stefan2
URL: <a class="moz-txt-link-freetext" \
                href="http://svn.apache.org/r1544736">http://svn.apache.org/r1544736</a>
                
* subversion/libsvn_ra_svn/marshal.c
   (svn_ra_svn__handle_command): code cleanup. No functional change
</pre>
        </blockquote>
        <pre wrap="">
Your change modifies the *terminate output parameter even if the
function fails. We have a long-standing policy against doing that.
You're taking too much for granted.
</pre>
      </blockquote>
      <pre wrap="">
We have a general policy, as I understand it, that if a function returns an error \
then we don't promise anything about what else it has done. On that basis, this \
change is fine. Is there some local policy overriding that general one in this \
case?</pre>  </blockquote>
    <br>
    Hm .... I remember differently. Not touching output parameters in
    the case of an error is a very nice property to have in an API (it
    makes debugging just that little bit easier, for example). But now I
    can't find an explicit statement to this effect in HACKING. On the
    other hand, we don't document that values of output parameters are
    indeterminate unless the function succeeds.<br>
    <br>
    Oh well. I knew senility would kick in at some point, I just didn't
    expect it so soon.<br>
    <br>
    -- Brane<br>
    <br>
    <div class="moz-signature">-- <br>
      <span style="font-face:sans-serif;font-size:9pt;line-height:18pt">Branko
        Čibej <span style="color: #f90">|</span> <span
          style="font-weight:bold">Director of Subversion</span>
        <br>
        WANdisco <span style="color: #f90">//</span> <span
          style="font-style:oblique">Non-Stop Data</span>
        <br>
        <span style="color: #ccc">e.</span> <a class="moz-txt-link-abbreviated" \
href="mailto:brane@wandisco.com">brane@wandisco.com</a></span></div>  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic