SourceForge has been redesigned. Learn more.
Close

#32 Patches to improve the code

main
closed-accepted
unSpawn
rkhunter (35)
5
2009-02-27
2009-02-16
No

Hi,

You will find attached several patches written by Jörg Sommer <joerg [at] alea.gnuu.de> which has improved the code.

Cheers,
Julien

Discussion

  • Julien Valroff

    Julien Valroff - 2009-02-16

    Patches from Jörg Sommer

     
  • unSpawn

    unSpawn - 2009-02-17

    Thanks. After a quick scan I think 1, 2 and 3 can be tested but for instance patch 4 may not be compatible with some OSes (AFAIK not all 'expr' have "match") and patch 6 is not Bourne compatible. (Deities know I would be the *first* one to allow myself the benefits of BAsh-isms). In any case we'll definately test these patches.

     
  • unSpawn

    unSpawn - 2009-02-17
    • labels: --> rkhunter
    • milestone: --> main
    • assigned_to: nobody --> unspawn
     
  • Julien Valroff

    Julien Valroff - 2009-02-17

    Thanks for your work!

    Julien

     
  • John Horne

    John Horne - 2009-02-17

    Patch 3 is wrong. Under Solaris 'which' return code does not state whether the command exists or not.
    The patch must not be applied.

     
  • John Horne

    John Horne - 2009-02-17

    Patch 2 cannot be applied since Solaris grep has no '-q' option. The patch must not be applied.

     
  • John Horne

    John Horne - 2009-02-17

    As with unSpawn patch 1 seems fine.
    Patch 4 may be possible. It works under Solaris, but would need testing on BSD at least (I have openbsd, and soon netbsd again at work). I would however perhaps suggest using the ':' convention rather than 'match' since ':' seems to be more known.
    Patch 5 is also possible, but I see no real advantage except perhaps in the display and suspscan functions. The fact that 1 pipe is/is not used in a command sequence is not going to be noticed by anyone. A pipe in a loop that is executed many times would be more of a consideration - the display function is executed many times as a whole, whereas in suspscan the use of sed in a loop may be worthwhile.
    Patch 6 would not normally work. However, since the change is only made in the BSD section, it is possible that it could be used.

    As unSpawn said, some patches are possible but would require testing under other OSes.

     
  • John Horne

    John Horne - 2009-02-17

    Patch 4: As suspected we would have to use the ':' operator rather than 'match'. Openbsd does not like match.

     
  • Julien Valroff

    Julien Valroff - 2009-02-18

    Thanks again for your work.

    To sum up:
    * Patch 1 can be applied
    * Patches 2 & 3 cannot as they do not work on Solaris
    * Patch 4 needs to be amended to use ':' as match is not everywhere
    * Patch 5 is ok but not very useful ATM - I think however it should be applied, don't you?
    * Patch 6 needs to be test on BSD

    Cheers,
    Julien

     
  • unSpawn

    unSpawn - 2009-02-23

    Current status:
    Patch 1 applied, testing locally before committing. Should be OK though.
    Patch 2 rejected: Solaris.
    Patch 3 rejected: Solaris.
    Patch 4 rejected: 'echo' being a builtin in Bash, Ksh (and Jsh, Ash, Bsh) so only benefit is where it's not (Tcsh, Csh?). It also fails to patch --fuzz cleanly.
    Patch 5 rejected: not a major improvement in terms of speed or otherwise.
    Patch 6 rejected: fails Heirloom* Sh: no variable substring ops. It also fails to patch --fuzz cleanly.

    * Gunnar Ritter, strict SVR4/SVID3 level shell, used for compatibility testing.

    Regards, unSpawn
    ---

     
  • unSpawn

    unSpawn - 2009-02-27
    • status: open --> closed-accepted
     
  • unSpawn

    unSpawn - 2009-02-27

    Thanks for submitting the patches. Commit done.

    Regards, unSpawn

     

Log in to post a comment.