Thread: Patch to prevent an accidental close of the plotting window

Started: 2008-09-11 00:31:25
Last activity: 2008-09-12 04:37:34
Topics: SAC Developers
Hi all,

BUG: When a user accidentally closes the plotting window, SAC will crash.

$ sac
SEISMIC ANALYSIS CODE [09/04/2008 (Version 101.2)]
Copyright 1995 Regents of the University of California

SAC> fg seis
SAC> p
[ ... closing plotting window ... ]
SAC> Seismic Analysis Code (SAC): Fatal IO Error 11 (Resource
temporarily unavailable) on the X server

Attached please find a patch for SAC 101.2 (provided by George
Helffrich and adapted a little by me) to prevent this by disabling the
"X" (close window) button of the plotting window. To apply this patch:

$ cd /path/to/sac
$ patch -p1 < prevent-accidental-close-of-plotting-window.diff

Best regards,

--
Kuang He
Department of Physics
University of Connecticut
Storrs, CT 06269-3046

Tel: +1.860.486.4919
Web: http://www.phys.uconn.edu/~he/

  • Kuang He,

    I got your patches applied to code base and quickly checked them.
    I received five (5) bugs from you, correct ?

    - Ctrl-D - This appears to work correctly.
    - X11 Closing window - This appears to disable the "close" window button
    At least on my mac. Was this the intention. Or did you want it to
    do something like "enddevice x"
    - evaluate bug - This works as well. I think there is something else
    going on here
    Try running the command twice, The message is slightly different
    between the first and second time.
    - sachistory - Easy fix, but important
    - tolower/toupper - This needs to be pulled out and checked for at ./
    configure time

    Side note:
    When you compile with the --enable-readline does it actually use the
    readline library or a libedit library.
    If you are using a readline library, could you check running sac
    within a script to see how it behaves.

    Cheers
    Brian

    On Sep 10, 2008, at 5:31 PM , Kuang He wrote:

    Hi all,

    BUG: When a user accidentally closes the plotting window, SAC will
    crash.

    $ sac
    SEISMIC ANALYSIS CODE [09/04/2008 (Version 101.2)]
    Copyright 1995 Regents of the University of California

    SAC> fg seis
    SAC> p
    [ ... closing plotting window ... ]
    SAC> Seismic Analysis Code (SAC): Fatal IO Error 11 (Resource
    temporarily unavailable) on the X server

    Attached please find a patch for SAC 101.2 (provided by George
    Helffrich and adapted a little by me) to prevent this by disabling the
    "X" (close window) button of the plotting window. To apply this patch:

    $ cd /path/to/sac
    $ patch -p1 < prevent-accidental-close-of-plotting-window.diff

    Best regards,

    --
    Kuang He
    Department of Physics
    University of Connecticut
    Storrs, CT 06269-3046

    Tel: +1.860.486.4919
    Web: http://www.phys.uconn.edu/~he/<prevent-accidental-close-of-
    plotting-window.diff>_______________________________________________
    sac-dev mailing list
    sac-dev<at>iris.washington.edu
    http://www.iris.washington.edu/mailman/listinfo/sac-dev


    • Dear Brian,

      On Thu, Sep 11, 2008 at 5:49 PM, Brian Savage <savage<at>uri.edu> wrote:
      Kuang He,

      I got your patches applied to code base and quickly checked them.
      I received five (5) bugs from you, correct ?

      Yes, that is correct.

      - Ctrl-D - This appears to work correctly.
      - X11 Closing window - This appears to disable the "close" window button
      At least on my mac. Was this the intention. Or did you want it to
      do something like "enddevice x"

      That behavior is intended by George's patch since he said he did not
      want the action to result in the close of all the windows, but
      actually what I want it to do is something like "ed x".

      When I was going through the archives of sac-dev yesterday, I found
      that you did have provided a patch to do exactly that back in 2005,
      but it seems that patch never got merged into the code base. Maybe we
      could reuse that patch. What do you think?

      http://www.iris.washington.edu/pipermail/sac-dev/2005-October/000000.html

      - evaluate bug - This works as well. I think there is something else going
      on here
      Try running the command twice, The message is slightly different
      between the first and second time.

      I've noticed this and also suspect that there is something else going
      on here. The code snippets I've commented out in the post

      http://www.iris.washington.edu/pipermail/sac-dev/2008-September/000086.html

      do not seem to have double free problem to me. Yet commenting them out
      can solve the problem. I don't know whether it is just glibc that is
      being paranoid.

      George mentioned the below to me in an email.

      Here is the Fortran code for that part of the program, perhaps
      slightly more illuminating as to what it is doing:
      -----
      nchar=nint(flnum(j+1))
      iend=ibeg+nchar+2
      if(iend.le.mcmsg)then
      kmsg(ibeg:ibeg)=kval(1:1)
      call copykc(j+2,nchar,kmsg(ibeg+1:))
      kmsg(iend-1:iend-1)=kval(1:1)
      kmsg(iend:iend)=' '
      if(j.eq.jcom)iarrow=ibeg
      ibeg=iend+1
      endif
      -----
      Here, the call to copykc moves the text of the message from the
      tokenized parse text (indexed by j, and implicitly used by copykc)
      into the output message text. Commenting that part out may fix a
      double free, but it also eliminates the message! However, it
      looks like the calling sequence for copykc is different, so there
      might have been subsequent changes to the C code.

      I don't know whether you have the history of that (perhaps quite old)
      change in the C code in the CVS. If you do, we might be able to figure
      out what is going on.

      - sachistory - Easy fix, but important
      - tolower/toupper - This needs to be pulled out and checked for at
      ./configure time

      Side note:
      When you compile with the --enable-readline does it actually use the
      readline library or a libedit library.
      If you are using a readline library, could you check running sac within a
      script to see how it behaves.

      Yes, when I compiled with --enable-readline, it did end up with using
      readline library instead of libedit.

      $ ldd $HOME/bin/sac | grep readline
      libreadline.so.5 => /lib/libreadline.so.5 (0xb7e66000)

      I tried to run sac within a shell script, and it works OK.

      $ cat run
      #!/bin/sh
      sac tt

      $ cat tt
      fg seis
      w test
      quit

      $ rm test && ./run
      SEISMIC ANALYSIS CODE [09/08/2008 (Version 101.2)]
      Copyright 1995 Regents of the University of California

      $ ll test
      -rw-rw-rw- 1 icrazy icrazy 4.6K Sep 11 18:44 test

      However, if I feed something to SAC through redirection, the one
      compiled with libedit works OK, but the one compiled with readline has
      the following problem.

      $ sac tt
      [ ... Works OK ... ]

      $ sac < tt
      SEISMIC ANALYSIS CODE [09/08/2008 (Version 101.2)]
      Copyright 1995 Regents of the University of California

      SAC> readline: readline_callback_read_char() called with no handler!
      Aborted

      Best regards,

      --
      Kuang He
      Department of Physics
      University of Connecticut
      Storrs, CT 06269-3046

      Tel: +1.860.486.4919
      Web: http://www.phys.uconn.edu/~he/

      • Kuang He

        Delete window:
        I think that old patch could be used to get the behavior we are
        expecting.

        Evaluate:
        The code George sent to you looks like a good place to start. I think
        the code is building an error message, which could be done rather
        easily.

        Readline and scripts:
        Looks like we need to do some exploration into this and possibly look
        into checking if we are using readline or libedit. Either readline and
        libedit react differently when run within a script or there might be a
        way to make them behave the same way.

        Cheers
        Brian

        On Sep 11, 2008, at 7:16 PM, "Kuang He" <icrazy<at>gmail.com> wrote:

        Dear Brian,

        On Thu, Sep 11, 2008 at 5:49 PM, Brian Savage <savage<at>uri.edu> wrote:
        Kuang He,

        I got your patches applied to code base and quickly checked them.
        I received five (5) bugs from you, correct ?

        Yes, that is correct.

        - Ctrl-D - This appears to work correctly.
        - X11 Closing window - This appears to disable the "close" window
        button
        At least on my mac. Was this the intention. Or did you want
        it to
        do something like "enddevice x"

        That behavior is intended by George's patch since he said he did not
        want the action to result in the close of all the windows, but
        actually what I want it to do is something like "ed x".

        When I was going through the archives of sac-dev yesterday, I found
        that you did have provided a patch to do exactly that back in 2005,
        but it seems that patch never got merged into the code base. Maybe we
        could reuse that patch. What do you think?

        http://www.iris.washington.edu/pipermail/sac-dev/2005-October/000000.html

        - evaluate bug - This works as well. I think there is something
        else going
        on here
        Try running the command twice, The message is slightly
        different
        between the first and second time.

        I've noticed this and also suspect that there is something else going
        on here. The code snippets I've commented out in the post

        http://www.iris.washington.edu/pipermail/sac-dev/2008-September/000086.html

        do not seem to have double free problem to me. Yet commenting them out
        can solve the problem. I don't know whether it is just glibc that is
        being paranoid.

        George mentioned the below to me in an email.

        Here is the Fortran code for that part of the program, perhaps
        slightly more illuminating as to what it is doing:
        -----
        nchar=nint(flnum(j+1))
        iend=ibeg+nchar+2
        if(iend.le.mcmsg)then
        kmsg(ibeg:ibeg)=kval(1:1)
        call copykc(j+2,nchar,kmsg(ibeg+1:))
        kmsg(iend-1:iend-1)=kval(1:1)
        kmsg(iend:iend)=' '
        if(j.eq.jcom)iarrow=ibeg
        ibeg=iend+1
        endif
        -----
        Here, the call to copykc moves the text of the message from the
        tokenized parse text (indexed by j, and implicitly used by copykc)
        into the output message text. Commenting that part out may fix a
        double free, but it also eliminates the message! However, it
        looks like the calling sequence for copykc is different, so there
        might have been subsequent changes to the C code.

        I don't know whether you have the history of that (perhaps quite old)
        change in the C code in the CVS. If you do, we might be able to figure
        out what is going on.

        - sachistory - Easy fix, but important
        - tolower/toupper - This needs to be pulled out and checked for at
        ./configure time

        Side note:
        When you compile with the --enable-readline does it actually use the
        readline library or a libedit library.
        If you are using a readline library, could you check running sac
        within a
        script to see how it behaves.

        Yes, when I compiled with --enable-readline, it did end up with using
        readline library instead of libedit.

        $ ldd $HOME/bin/sac | grep readline
        libreadline.so.5 => /lib/libreadline.so.5 (0xb7e66000)

        I tried to run sac within a shell script, and it works OK.

        $ cat run
        #!/bin/sh
        sac tt

        $ cat tt
        fg seis
        w test
        quit

        $ rm test && ./run
        SEISMIC ANALYSIS CODE [09/08/2008 (Version 101.2)]
        Copyright 1995 Regents of the University of California

        $ ll test
        -rw-rw-rw- 1 icrazy icrazy 4.6K Sep 11 18:44 test

        However, if I feed something to SAC through redirection, the one
        compiled with libedit works OK, but the one compiled with readline has
        the following problem.

        $ sac tt
        [ ... Works OK ... ]

        $ sac < tt
        SEISMIC ANALYSIS CODE [09/08/2008 (Version 101.2)]
        Copyright 1995 Regents of the University of California

        SAC> readline: readline_callback_read_char() called with no handler!
        Aborted

        Best regards,

        --
        Kuang He
        Department of Physics
        University of Connecticut
        Storrs, CT 06269-3046

        Tel: +1.860.486.4919
        Web: http://www.phys.uconn.edu/~he/
        _______________________________________________
        sac-dev mailing list
        sac-dev<at>iris.washington.edu
        http://www.iris.washington.edu/mailman/listinfo/sac-dev

23:59:47 v.22510d55