Move sha1 to C?

Move sha1 to C?

[Leo]

Tested on GNU Emacs 23.3.50.1 (x86_64-apple-darwin10.7.0, Carbon Version
1.6.0 AppKit 1038.35)

  (benchmark-run 10 (sha1 (make-string 1000000 ?a)))

Elisp: (76.100951 10 0.15035599999999996)

C:     (0.279209  10 0.14277499999999999)

Leo

Re: Move sha1 to C?

[Stefan Monnier]

I'm not sure if we discussed it in the past, but I wouldn't mind using
sha1 from a C library.


        Stefan

Re: Move sha1 to C?

[Leo]

On 2011-05-23 03:04 +0800, Stefan Monnier wrote:
> I'm not sure if we discussed it in the past, but I wouldn't mind using
> sha1 from a C library.

Excellent.

Do you mind if I do the following:

  1. add sha1.[ch] from gnulib
  2. export sha1 to elisp
  3. obsolete sha1.el

Should the signature of sha1 stay as is or change to be similar to md5?

   (md5 OBJECT &optional START END CODING-SYSTEM NOERROR)

Leo

Re: Move sha1 to C?

[Stefan Monnier]

> Do you mind if I do the following:

>   1. add sha1.[ch] from gnulib
>   2. export sha1 to elisp
>   3. obsolete sha1.el

Fine by me.  3 should also include "gut sha1.el's content" so that
loading it won't end up overwriting the C implementation with the old
Elisp code.

> Should the signature of sha1 stay as is or change to be similar to md5?
>    (md5 OBJECT &optional START END CODING-SYSTEM NOERROR)

I don't have any opinion on this.  The only thing I consider important
is to signal an error if the hashed text contains any
non-byte character.


        Stefan

Re: Move sha1 to C?

[Leo]

On 2011-05-23 09:23 +0800, Stefan Monnier wrote:
> Fine by me.  3 should also include "gut sha1.el's content" so that
> loading it won't end up overwriting the C implementation with the old
> Elisp code.

In the patch attached in last post, feature sha1 is provided so
(require 'sha1) should not load sha1.el but if people insist in using
(load "sha1") then the primitive is overwritten. I hope this is OK.

>
>> Should the signature of sha1 stay as is or change to be similar to md5?
>>    (md5 OBJECT &optional START END CODING-SYSTEM NOERROR)
>
> I don't have any opinion on this.  The only thing I consider important
> is to signal an error if the hashed text contains any
> non-byte character.

OK.

Leo

Re: Move sha1 to C?

[Stefan Monnier]

> In the patch attached in last post, feature sha1 is provided so
> (require 'sha1) should not load sha1.el but if people insist in using
> (load "sha1") then the primitive is overwritten. I hope this is OK.

We can completely remove sha1.el then.


        Stefan

Re: Move sha1 to C?

[Leo]

On 2011-05-23 20:04 +0800, Stefan Monnier wrote:
>> In the patch attached in last post, feature sha1 is provided so
>> (require 'sha1) should not load sha1.el but if people insist in using
>> (load "sha1") then the primitive is overwritten. I hope this is OK.
>
> We can completely remove sha1.el then.

I like this.

vc-bzr.el uses the variable sha1-program. So I am thinking of moving it
to vc.el as shown in the attached patch
(full patch in http://paste.pocoo.org/show/393652).

BTW, what to do with auto-generated files and files from gnulib when
committing?

Leo

--- lisp/vc/vc-bzr.el	2011-03-12 15:26:33 +0000
+++ lisp/vc/vc-bzr.el	2011-05-23 13:07:11 +0000
@@ -156,12 +156,10 @@
 	(push (cons (match-string 1) (match-string 2)) settings)))
     settings))
 
-(require 'sha1)                         ;For sha1-program
-
 (defun vc-bzr-sha1 (file)
   (with-temp-buffer
     (set-buffer-multibyte nil)
-    (let ((prog sha1-program)
+    (let ((prog vc-sha1-program)
           (args nil)
 	  process-file-side-effects)
       (when (consp prog)

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2011-04-20 23:34:00 +0000
+++ lisp/vc/vc.el	2011-05-23 13:06:39 +0000
@@ -752,6 +752,14 @@
   :group 'vc
   :version "22.1")
 
+(defcustom vc-sha1-program '("sha1sum")
+  "Name of program to compute SHA1.
+It must be a string \(program name\) or list of strings \(name and its args\)."
+  :type '(repeat string)
+  :group 'vc)
+
+(define-obsolete-variable-alias 'sha1-program 'vc-sha1-program "24.1")
+
 ;;;###autoload
 (defcustom vc-checkout-hook nil
   "Normal hook (list of functions) run after checking out a file.

Re: Move sha1 to C?

[Stefan Monnier]

> vc-bzr.el uses the variable sha1-program. So I am thinking of moving it
> to vc.el as shown in the attached patch
> (full patch in http://paste.pocoo.org/show/393652).

I'd put it in vc-bzr.el instead.

> BTW, what to do with auto-generated files and files from gnulib when
> committing?

Can you be more specific?


        Stefan

Re: Move sha1 to C?

[Leo]

On 2011-05-23 21:27 +0800, Stefan Monnier wrote:
>> vc-bzr.el uses the variable sha1-program. So I am thinking of moving it
>> to vc.el as shown in the attached patch
>> (full patch in http://paste.pocoo.org/show/393652).
>
> I'd put it in vc-bzr.el instead.

OK.

>> BTW, what to do with auto-generated files and files from gnulib when
>> committing?
>
> Can you be more specific?
>
>
>         Stefan

I change Makefile.in in topdir to include crypto/sha1 from gnulib and
run ./configure and make sync-from-gnulib and this introduces lots of
changes, specifically those marked with * in the following list:

    edited               Makefile.in
                         lib/
*   edited               lib/gnulib.mk
*   unregistered         lib/sha1.c
*   unregistered         lib/sha1.h
*   edited               lib/verify.h
                         lisp/
    edited               lisp/bindings.el
    removed              lisp/sha1.el
                         lisp/vc/
    edited               lisp/vc/vc-bzr.el
    edited               lisp/vc/vc.el
                         m4/
*   edited               m4/gl-comp.m4
*   unregistered         m4/sha1.m4
                         src/
    edited               src/deps.mk
    edited               src/fns.c

So when committing what to do with those changes?

Leo

Re: Move sha1 to C?

[Stefan Monnier]

> I change Makefile.in in topdir to include crypto/sha1 from gnulib and
> run ./configure and make sync-from-gnulib and this introduces lots of
> changes, specifically those marked with * in the following list:

>     edited               Makefile.in
>                          lib/
> *   edited               lib/gnulib.mk
> *   unregistered         lib/sha1.c
> *   unregistered         lib/sha1.h
> *   edited               lib/verify.h
>                          lisp/
>     edited               lisp/bindings.el
>     removed              lisp/sha1.el
>                          lisp/vc/
>     edited               lisp/vc/vc-bzr.el
>     edited               lisp/vc/vc.el
>                          m4/
> *   edited               m4/gl-comp.m4
> *   unregistered         m4/sha1.m4
>                          src/
>     edited               src/deps.mk
>     edited               src/fns.c

> So when committing what to do with those changes?

Since these all seem to be related to the addition of the sha1 module
from gnulib (as opposed to being unrelated changes brought in by the
"sync with latest gnulib" that happened as a side-effect), I'd include
them in the commit in the normal way (i.e. more or less as if you'd
done those changes by hand, tho the ChangeLog can be a bit more concise).


        Stefan

Re: Move sha1 to C?

[Leo]

CC'd Paul for help.


On 2011-05-23 21:57 +0800, Stefan Monnier wrote:
[snipped 26 lines]
> Since these all seem to be related to the addition of the sha1 module
> from gnulib (as opposed to being unrelated changes brought in by the
> "sync with latest gnulib" that happened as a side-effect), I'd include
> them in the commit in the normal way (i.e. more or less as if you'd
> done those changes by hand, tho the ChangeLog can be a bit more concise).
>
>
>         Stefan

I run gnulib-tool on OSX and get tons of lines such as:

  sed: 1: "/^ *unistd *$/{s/^.*$/t ...": bad flag in substitute command: '}'
  ../gnulib/gnulib-tool: line 801: printf: write error: Broken pipe

In addition, the auto-generated files seem to have changes unrelated to
sha1 (see the example attached to the end).

What to do?

Leo

=== modified file 'lib/gnulib.mk'
--- lib/gnulib.mk	2011-05-18 00:39:40 +0000
+++ lib/gnulib.mk	2011-05-23 07:22:44 +0000
@@ -9,7 +9,7 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=. --makefile-name=gnulib.mk --no-libtool --macro-prefix=gl --no-vc-files careadlinkat crypto/md5 dtoastr filemode getloadavg getopt-gnu ignore-value intprops lstat mktime readlink socklen stdarg stdio strftime strtoumax symlink sys_stat
+# Reproduce by: gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=. --makefile-name=gnulib.mk --no-libtool --macro-prefix=gl --no-vc-files careadlinkat crypto/md5 crypto/sha1 dtoastr filemode getloadavg getopt-gnu ignore-value intprops lstat mktime readlink socklen stdarg stdio strftime strtoumax symlink sys_stat
 
 
 MOSTLYCLEANFILES += core *.stackdump
@@ -94,11 +94,18 @@
 
 ## end   gnulib module crypto/md5
 
+## begin gnulib module crypto/sha1
+
+
+EXTRA_DIST += sha1.c sha1.h
+
+EXTRA_libgnu_a_SOURCES += sha1.c
+
+## end   gnulib module crypto/sha1
+
 ## begin gnulib module dosname
 
-if gl_GNULIB_ENABLED_dosname
 
-endif
 EXTRA_DIST += dosname.h
 
 ## end   gnulib module dosname
@@ -159,10 +166,8 @@
 
 ## begin gnulib module gettext-h
 
-if gl_GNULIB_ENABLED_be453cec5eecf5731a274f2de7f2db36
 libgnu_a_SOURCES += gettext.h
 
-endif
 ## end   gnulib module gettext-h
 
 ## begin gnulib module ignore-value
@@ -250,9 +255,7 @@
 
 ## begin gnulib module stat
 
-if gl_GNULIB_ENABLED_stat
 
-endif
 EXTRA_DIST += stat.c
 
 EXTRA_libgnu_a_SOURCES += stat.c
@@ -610,9 +613,7 @@
 
 ## begin gnulib module strtoull
 
-if gl_GNULIB_ENABLED_strtoull
 
-endif
 EXTRA_DIST += strtol.c strtoul.c strtoull.c
 
 EXTRA_libgnu_a_SOURCES += strtol.c strtoul.c strtoull.c
@@ -885,9 +886,7 @@
 
 ## begin gnulib module verify
 
-if gl_GNULIB_ENABLED_verify
 
-endif
 EXTRA_DIST += verify.h
 
 ## end   gnulib module verify

=== modified file 'm4/gl-comp.m4'
--- m4/gl-comp.m4	2011-05-04 07:19:21 +0000
+++ m4/gl-comp.m4	2011-05-23 18:01:16 +0000
@@ -31,6 +31,7 @@
   # Code from module c++defs:
   # Code from module careadlinkat:
   # Code from module crypto/md5:
+  # Code from module crypto/sha1:
   # Code from module dosname:
   # Code from module dtoastr:
   # Code from module extensions:
@@ -92,6 +93,7 @@
   gl_source_base='lib'
 AC_CHECK_FUNCS_ONCE([readlinkat])
 gl_MD5
+gl_SHA1
 AC_REQUIRE([gl_C99_STRTOLD])
 gl_FILEMODE
 gl_GETLOADAVG([$gl_source_base])
@@ -99,6 +101,8 @@
 gl_FUNC_GETOPT_GNU
 gl_MODULE_INDICATOR_FOR_TESTS([getopt-gnu])
 gl_FUNC_GETOPT_POSIX
+AC_SUBST([LIBINTL])
+AC_SUBST([LTLIBINTL])
 AC_REQUIRE([AC_C_INLINE])
 gl_INTTYPES_INCOMPLETE
 gl_FUNC_LSTAT
@@ -110,6 +114,8 @@
 gl_UNISTD_MODULE_INDICATOR([readlink])
 gl_TYPE_SOCKLEN_T
 gt_TYPE_SSIZE_T
+gl_FUNC_STAT
+gl_SYS_STAT_MODULE_INDICATOR([stat])
 gl_STDARG_H
 AM_STDBOOL_H
 gl_STDDEF_H
@@ -117,6 +123,8 @@
 gl_STDIO_H
 gl_STDLIB_H
 gl_FUNC_GNU_STRFTIME
+gl_FUNC_STRTOULL
+gl_STDLIB_MODULE_INDICATOR([strtoull])
 gl_FUNC_STRTOUMAX
 gl_INTTYPES_MODULE_INDICATOR([strtoumax])
 gl_FUNC_SYMLINK
@@ -127,74 +135,7 @@
 gl_TIME_R
 gl_TIME_MODULE_INDICATOR([time_r])
 gl_UNISTD_H
-  gl_gnulib_enabled_dosname=false
-  gl_gnulib_enabled_be453cec5eecf5731a274f2de7f2db36=false
-  gl_gnulib_enabled_stat=false
-  gl_gnulib_enabled_strtoull=false
-  gl_gnulib_enabled_verify=false
-  func_gl_gnulib_m4code_dosname ()
-  {
-    if ! $gl_gnulib_enabled_dosname; then
-      gl_gnulib_enabled_dosname=true
-    fi
-  }
-  func_gl_gnulib_m4code_be453cec5eecf5731a274f2de7f2db36 ()
-  {
-    if ! $gl_gnulib_enabled_be453cec5eecf5731a274f2de7f2db36; then
-AC_SUBST([LIBINTL])
-AC_SUBST([LTLIBINTL])
-      gl_gnulib_enabled_be453cec5eecf5731a274f2de7f2db36=true
-    fi
-  }
-  func_gl_gnulib_m4code_stat ()
-  {
-    if ! $gl_gnulib_enabled_stat; then
-gl_FUNC_STAT
-gl_SYS_STAT_MODULE_INDICATOR([stat])
-      gl_gnulib_enabled_stat=true
-      if $condition; then
-        func_gl_gnulib_m4code_dosname
-      fi
-    fi
-  }
-  func_gl_gnulib_m4code_strtoull ()
-  {
-    if ! $gl_gnulib_enabled_strtoull; then
-gl_FUNC_STRTOULL
-gl_STDLIB_MODULE_INDICATOR([strtoull])
-      gl_gnulib_enabled_strtoull=true
-    fi
-  }
-  func_gl_gnulib_m4code_verify ()
-  {
-    if ! $gl_gnulib_enabled_verify; then
-      gl_gnulib_enabled_verify=true
-    fi
-  }
-  if test $GNULIB_UNISTD_H_GETOPT = 1; then
-    func_gl_gnulib_m4code_be453cec5eecf5731a274f2de7f2db36
-  fi
-  if test $REPLACE_LSTAT = 1; then
-    func_gl_gnulib_m4code_dosname
-  fi
-  if test $REPLACE_LSTAT = 1; then
-    func_gl_gnulib_m4code_stat
-  fi
-  if test $HAVE_READLINK = 0 || test $REPLACE_READLINK = 1; then
-    func_gl_gnulib_m4code_stat
-  fi
-  if test "$ac_cv_have_decl_strtoumax" != yes && test $ac_cv_func_strtoumax = no; then
-    func_gl_gnulib_m4code_verify
-  fi
-  if test "$ac_cv_have_decl_strtoumax" != yes && test $ac_cv_func_strtoumax = no && test $ac_cv_type_unsigned_long_long_int = yes; then
-    func_gl_gnulib_m4code_strtoull
-  fi
   m4_pattern_allow([^gl_GNULIB_ENABLED_])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_dosname], [$gl_gnulib_enabled_dosname])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_be453cec5eecf5731a274f2de7f2db36], [$gl_gnulib_enabled_be453cec5eecf5731a274f2de7f2db36])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_stat], [$gl_gnulib_enabled_stat])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_strtoull], [$gl_gnulib_enabled_strtoull])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_verify], [$gl_gnulib_enabled_verify])
   # End of code from modules
   m4_ifval(gl_LIBSOURCES_LIST, [
     m4_syscmd([test ! -d ]m4_defn([gl_LIBSOURCES_DIR])[ ||
@@ -364,6 +305,8 @@
   lib/mktime-internal.h
   lib/mktime.c
   lib/readlink.c
+  lib/sha1.c
+  lib/sha1.h
   lib/stat.c
   lib/stdarg.in.h
   lib/stdbool.in.h
@@ -399,6 +342,7 @@
   m4/mktime.m4
   m4/multiarch.m4
   m4/readlink.m4
+  m4/sha1.m4
   m4/socklen.m4
   m4/ssize_t.m4
   m4/st_dm_mode.m4

Re: Move sha1 to C?

[Stefan Monnier]

> I run gnulib-tool on OSX and get tons of lines such as:

>   sed: 1: "/^ *unistd *$/{s/^.*$/t ...": bad flag in substitute command: '}'
>   ../gnulib/gnulib-tool: line 801: printf: write error: Broken pipe

Can't help you there.

> In addition, the auto-generated files seem to have changes unrelated to
> sha1 (see the example attached to the end).

Then install the result of the gnulib-sync as a separate commit.


        Stefan

Re: Move sha1 to C?

[Paul Eggert]

[Adding bug-gnulib to the CC: list.]

On 05/23/11 11:14, Leo wrote:

> I run gnulib-tool on OSX and get tons of lines such as:
> 
>   sed: 1: "/^ *unistd *$/{s/^.*$/t ...": bad flag in substitute command: '}'
>   ../gnulib/gnulib-tool: line 801: printf: write error: Broken pipe

My guess is that's due to a bit of unportable code in gnulib-tool,
which I just fixed as follows.  Could you please try it again?  If
that doesn't work, I can push the sha1 gnulib code into the Emacs
trunk myself, but I'd rather get it working for everybody.  Thanks.

---
 ChangeLog   |    8 ++++++++
 gnulib-tool |    8 ++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index de4c489..3770526 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-05-23  Paul Eggert  <eggert@cs.ucla.edu>
+
+	gnulib-tool: fix portability problem with MacOS sed
+	A sed command like "/x/{s/a/b/}" is not portable; a newline is needed
+	before the "}".  Problem reported by Leo in
+	<http://lists.gnu.org/archive/html/emacs-devel/2011-05/msg00717.html>.
+	* gnulib-tool (sed_dependencies_without_conditions):
+
 2011-05-23  Bruno Haible  <bruno@clisp.org>
 
 	getugroups: Fix module description.
diff --git a/gnulib-tool b/gnulib-tool
index f6c29f2..0bec3b3 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -2694,8 +2694,12 @@ func_modules_transitive_closure ()
               func_append inmodules " $dep"
               if test -n "$cond_dependencies"; then
                 escaped_dep=`echo "$dep" | sed -e "$sed_escape_dependency"`
-                sed_extract_condition1='/^ *'"$escaped_dep"' *$/{s/^.*$/true/p}'
-                sed_extract_condition2='/^ *'"$escaped_dep"' *\[.*\] *$/{s/^ *'"$escaped_dep"' *\[\(.*\)\] *$/\1/p}'
+                sed_extract_condition1='/^ *'"$escaped_dep"' *$/{
+                  s/^.*$/true/p
+                }'
+                sed_extract_condition2='/^ *'"$escaped_dep"' *\[.*\] *$/{
+                  s/^ *'"$escaped_dep"' *\[\(.*\)\] *$/\1/p
+                }'
                 condition=`func_get_dependencies $module | sed -n -e "$sed_extract_condition1" -e "$sed_extract_condition2"`
                 if test "$condition" = true; then
                   condition=
-- 
1.7.4.4

Re: Move sha1 to C?

[Leo]

On 2011-05-24 06:10 +0800, Paul Eggert wrote:
> My guess is that's due to a bit of unportable code in gnulib-tool,
> which I just fixed as follows.  Could you please try it again?  If
> that doesn't work, I can push the sha1 gnulib code into the Emacs
> trunk myself, but I'd rather get it working for everybody.  Thanks.

It works smoothly now. BTW, md5sum is not available on OSX. I alias md5
to md5sum and the script seems to run without any problem.

man md5:

----------------

SYNOPSIS
     md5 [-pqrtx] [-s string] [file ...]

DESCRIPTION
     The md5 utility takes as input a message of arbitrary length and produces
     as output a ``fingerprint'' or ``message digest'' of the input.  It is
     conjectured that it is computationally infeasible to produce two messages
     having the same message digest, or to produce any message having a given
     prespecified target message digest.  The MD5 algorithm is intended for
     digital signature applications, where a large file must be ``compressed''
     in a secure manner before being encrypted with a private (secret) key
     under a public-key cryptosystem such as RSA.

     MD5 has not yet (2001-09-03) been broken, but sufficient attacks have
     been made that its security is in some doubt.  The attacks on MD5 are in
     the nature of finding ``collisions'' -- that is, multiple inputs which
     hash to the same value; it is still unlikely for an attacker to be able
     to determine the exact original input given a hash value.

     The following options may be used in any combination and must precede any
     files named on the command line.  The hexadecimal checksum of each file
     listed on the command line is printed after the options are processed.

     -s string
             Print a checksum of the given string.

     -p      Echo stdin to stdout and append the checksum to stdout.

     -q      Quiet mode - only the checksum is printed out.  Overrides the -r
             option.

     -r      Reverses the format of the output.  This helps with visual diffs.
             Does nothing when combined with the -ptx options.

     -t      Run a built-in time trial.

     -x      Run a built-in test script.

Re: Move sha1 to C?

[Eli Zaretskii]

By the way, should we note in NEWS that sha1.el was deleted (without
losing functionality)?

Re: Move sha1 to C?

[Leo]

On 2011-05-24 18:01 +0800, Eli Zaretskii wrote:
> By the way, should we note in NEWS that sha1.el was deleted (without
> losing functionality)?

Done in revno: 104351.

** Function `sha1' is now implemented in C for speed.
The elisp implementation sha1.el is removed. Feature sha1 is provided
by default.

Leo

Re: Move sha1 to C?

[Richard Stallman]

    By the way, should we note in NEWS that sha1.el was deleted (without
    losing functionality)?

It is not useful to mention such minor things in NEWS.
If it is full of unimportant things, people will get in the
habit of not reading it.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

Re: Move sha1 to C?

[Thien-Thi Nguyen]

() Paul Eggert <eggert@cs.ucla.edu>
() Mon, 23 May 2011 15:10:47 -0700

   +                sed_extract_condition1='/^ *'"$escaped_dep"' *$/{
   +                  s/^.*$/true/p
   +                }'
   +                sed_extract_condition2='/^ *'"$escaped_dep"' *\[.*\] *$/{
   +                  s/^ *'"$escaped_dep"' *\[\(.*\)\] *$/\1/p
   +                }'

IIUC the curly braces are optional if they contain only one command.
Is that not the case everywhere?

Re: Move sha1 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Date: Mon, 23 May 2011 21:34:49 +0800
> Cc: emacs-devel@gnu.org
> 
>     edited               Makefile.in
>                          lib/
> *   edited               lib/gnulib.mk
> *   unregistered         lib/sha1.c
> *   unregistered         lib/sha1.h
> *   edited               lib/verify.h
>                          lisp/
>     edited               lisp/bindings.el
>     removed              lisp/sha1.el
>                          lisp/vc/
>     edited               lisp/vc/vc-bzr.el
>     edited               lisp/vc/vc.el
>                          m4/
> *   edited               m4/gl-comp.m4
> *   unregistered         m4/sha1.m4
>                          src/
>     edited               src/deps.mk
>     edited               src/fns.c

Don't you need to modify lib/Makefile.am as well?

Btw, if you can afford it, please consider making in
src/makefile.w32-in and in lib/makefile.w32-in the changes equivalent
to what you did in src/deps.mk and lib/gnulib.mk, respectively, to
prevent the Windows build from breaking.  TIA.

Re: Move sha1 to C?

[Leo]

On 2011-05-23 22:30 +0800, Eli Zaretskii wrote:
> Don't you need to modify lib/Makefile.am as well?

which includes gnulib.mk, which is updated automatically.

> Btw, if you can afford it, please consider making in
> src/makefile.w32-in and in lib/makefile.w32-in the changes equivalent
> to what you did in src/deps.mk and lib/gnulib.mk, respectively, to
> prevent the Windows build from breaking.  TIA.

Is the following sufficient?

=== modified file 'src/makefile.w32-in'
--- src/makefile.w32-in	2011-05-18 11:32:07 +0000
+++ src/makefile.w32-in	2011-05-23 17:46:52 +0000
@@ -866,6 +866,7 @@
 	$(EMACS_ROOT)/nt/inc/unistd.h \
 	$(EMACS_ROOT)/nt/inc/sys/time.h \
 	$(EMACS_ROOT)/lib/md5.h \
+	$(EMACS_ROOT)/lib/sha1.h \
 	$(LISP_H) \
 	$(SRC)/atimer.h \
 	$(SRC)/blockinput.h \

Leo

Re: Move sha1 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Date: Tue, 24 May 2011 01:47:47 +0800
> 
> > Btw, if you can afford it, please consider making in
> > src/makefile.w32-in and in lib/makefile.w32-in the changes equivalent
> > to what you did in src/deps.mk and lib/gnulib.mk, respectively, to
> > prevent the Windows build from breaking.  TIA.
> 
> Is the following sufficient?
> 
> === modified file 'src/makefile.w32-in'
> --- src/makefile.w32-in	2011-05-18 11:32:07 +0000
> +++ src/makefile.w32-in	2011-05-23 17:46:52 +0000
> @@ -866,6 +866,7 @@
>  	$(EMACS_ROOT)/nt/inc/unistd.h \
>  	$(EMACS_ROOT)/nt/inc/sys/time.h \
>  	$(EMACS_ROOT)/lib/md5.h \
> +	$(EMACS_ROOT)/lib/sha1.h \
>  	$(LISP_H) \
>  	$(SRC)/atimer.h \
>  	$(SRC)/blockinput.h \

For src/makefile.w32-in, yes.  For lib/makefile.w32-in, sha1.c should
have its prerequisites added, and sha1.$(O) added to GNULIBOBJS.

TIA

Re: Move sha1 to C?

[Leo]

On 2011-05-24 02:12 +0800, Eli Zaretskii wrote:
> For src/makefile.w32-in, yes.  For lib/makefile.w32-in, sha1.c should
> have its prerequisites added, and sha1.$(O) added to GNULIBOBJS.
>
> TIA

Something like the following?

=== modified file 'lib/makefile.w32-in'
--- lib/makefile.w32-in	2011-04-06 16:05:49 +0000
+++ lib/makefile.w32-in	2011-05-23 18:17:52 +0000
@@ -29,6 +29,7 @@
 	     $(BLD)/strftime.$(O) \
 	     $(BLD)/time_r.$(O) \
 	     $(BLD)/md5.$(O) \
+	     $(BLD)/sha1.$(O) \
 	     $(BLD)/filemode.$(O)
 
 #
@@ -110,6 +111,15 @@
 	$(EMACS_ROOT)/src/m/intel386.h \
 	$(EMACS_ROOT)/src/config.h
 
+$(BLD)/sha1.$(O) : \
+	$(SRC)/sha1.c \
+	$(SRC)/sha1.h \
+	$(EMACS_ROOT)/nt/inc/stdint.h \
+	$(EMACS_ROOT)/nt/inc/sys/stat.h \
+	$(EMACS_ROOT)/src/s/ms-w32.h \
+	$(EMACS_ROOT)/src/m/intel386.h \
+	$(EMACS_ROOT)/src/config.h
+
 $(BLD)/filemode.$(O) : \
 	$(SRC)/filemode.c \
 	$(SRC)/filemode.h \

=== modified file 'src/makefile.w32-in'
--- src/makefile.w32-in	2011-05-18 11:32:07 +0000
+++ src/makefile.w32-in	2011-05-23 17:46:52 +0000
@@ -866,6 +866,7 @@
 	$(EMACS_ROOT)/nt/inc/unistd.h \
 	$(EMACS_ROOT)/nt/inc/sys/time.h \
 	$(EMACS_ROOT)/lib/md5.h \
+	$(EMACS_ROOT)/lib/sha1.h \
 	$(LISP_H) \
 	$(SRC)/atimer.h \
 	$(SRC)/blockinput.h \

Re: Move sha1 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 24 May 2011 02:23:52 +0800
> 
> On 2011-05-24 02:12 +0800, Eli Zaretskii wrote:
> > For src/makefile.w32-in, yes.  For lib/makefile.w32-in, sha1.c should
> > have its prerequisites added, and sha1.$(O) added to GNULIBOBJS.
> >
> > TIA
> 
> Something like the following?

Yes, thanks.

Re: Move sha1 to C?

[Eli Zaretskii]

> From: Leo <sdl.web@gmail.com>
> Date: Mon, 23 May 2011 04:02:04 +0800
> 
>   1. add sha1.[ch] from gnulib

Note that gnulib's sha1.c uses malloc, which is not safe in Emacs.
So either make sure sha1_stream is never ever called by Emacs, or use
xmalloc instead.

> Should the signature of sha1 stay as is or change to be similar to md5?
> 
>    (md5 OBJECT &optional START END CODING-SYSTEM NOERROR)

Are there any real reasons not to keep the API?  If there are good
reasons, let's hear them.  If there are no good reasons, my vote is to
keep the API backward-compatible.

Re: Move sha1 to C?

[Leo]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On 2011-05-23 15:21 +0800, Eli Zaretskii wrote:
>> From: Leo <sdl.web@gmail.com>
>> Date: Mon, 23 May 2011 04:02:04 +0800
>> 
>>   1. add sha1.[ch] from gnulib
>
> Note that gnulib's sha1.c uses malloc, which is not safe in Emacs.
> So either make sure sha1_stream is never ever called by Emacs, or use
> xmalloc instead.

Thanks for this note. sha1_stream is not used.

>> Should the signature of sha1 stay as is or change to be similar to md5?
>> 
>>    (md5 OBJECT &optional START END CODING-SYSTEM NOERROR)
>
> Are there any real reasons not to keep the API?  If there are good
> reasons, let's hear them.  If there are no good reasons, my vote is to
> keep the API backward-compatible.

I think it is better to keep the API. The BINARY arg can be useful.

Please review the attached patch. ChangeLog entry omitted for now.

Changes due to 'make sync-from-gnulib' aren't included.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sha1.diff --]
[-- Type: text/x-diff, Size: 7840 bytes --]

=== modified file 'Makefile.in'
--- Makefile.in	2011-05-21 09:53:32 +0000
+++ Makefile.in	2011-05-23 08:00:38 +0000
@@ -331,8 +331,8 @@
 # $(gnulib_srcdir) (relative to $(srcdir) and should have build tools
 # as per $(gnulib_srcdir)/DEPENDENCIES.
 GNULIB_MODULES = \
-  careadlinkat crypto/md5 dtoastr filemode getloadavg getopt-gnu \
-  ignore-value intprops lstat mktime readlink \
+  careadlinkat crypto/md5 crypto/sha1 dtoastr filemode getloadavg \
+  getopt-gnu ignore-value intprops lstat mktime readlink \
   socklen stdarg stdio strftime strtoumax symlink sys_stat
 GNULIB_TOOL_FLAGS = \
  --conditional-dependencies --import --no-changelog --no-vc-files \

=== modified file 'lisp/bindings.el'
--- lisp/bindings.el	2011-04-19 13:44:55 +0000
+++ lisp/bindings.el	2011-05-23 08:02:18 +0000
@@ -646,9 +646,10 @@
 
 (make-variable-buffer-local 'indent-tabs-mode)
 
-;; We have base64 and md5 functions built in now.
+;; We have base64, md5 and sha1 functions built in now.
 (provide 'base64)
 (provide 'md5)
+(provide 'sha1)
 (provide 'overlay '(display syntax-table field))
 (provide 'text-properties '(display syntax-table field point-entered))
 

=== renamed file 'lisp/sha1.el' => 'lisp/obsolete/sha1.el'
--- lisp/sha1.el	2011-01-25 04:08:28 +0000
+++ lisp/obsolete/sha1.el	2011-05-23 08:07:18 +0000
@@ -4,6 +4,7 @@
 
 ;; Author: Shuhei KOBAYASHI <shuhei@aqua.ocn.ne.jp>
 ;; Keywords: SHA1, FIPS 180-1
+;; Obsolete-since: 24.1
 
 ;; This file is part of GNU Emacs.
 

=== modified file 'src/deps.mk'
--- src/deps.mk	2011-04-23 10:33:28 +0000
+++ src/deps.mk	2011-05-23 06:31:38 +0000
@@ -284,7 +284,8 @@
 floatfns.o: floatfns.c syssignal.h lisp.h globals.h $(config_h)
 fns.o: fns.c commands.h lisp.h $(config_h) frame.h buffer.h character.h \
    keyboard.h keymap.h window.h $(INTERVALS_H) coding.h ../lib/md5.h \
-   blockinput.h atimer.h systime.h xterm.h ../lib/unistd.h globals.h
+   ../lib/sha1.h blockinput.h atimer.h systime.h xterm.h ../lib/unistd.h \
+   globals.h
 print.o: print.c process.h frame.h window.h buffer.h keyboard.h character.h \
    lisp.h globals.h $(config_h) termchar.h $(INTERVALS_H) msdos.h termhooks.h \
    blockinput.h atimer.h systime.h font.h charset.h coding.h ccl.h \

=== modified file 'src/fns.c'
--- src/fns.c	2011-05-15 17:17:44 +0000
+++ src/fns.c	2011-05-23 06:20:26 +0000
@@ -4514,42 +4514,17 @@
 
 \f
 /************************************************************************
-				 MD5
+                             MD5 and SHA1
  ************************************************************************/
 
 #include "md5.h"
+#include "sha1.h"
 
-DEFUN ("md5", Fmd5, Smd5, 1, 5, 0,
-       doc: /* Return MD5 message digest of OBJECT, a buffer or string.
-
-A message digest is a cryptographic checksum of a document, and the
-algorithm to calculate it is defined in RFC 1321.
-
-The two optional arguments START and END are character positions
-specifying for which part of OBJECT the message digest should be
-computed.  If nil or omitted, the digest is computed for the whole
-OBJECT.
-
-The MD5 message digest is computed from the result of encoding the
-text in a coding system, not directly from the internal Emacs form of
-the text.  The optional fourth argument CODING-SYSTEM specifies which
-coding system to encode the text with.  It should be the same coding
-system that you used or will use when actually writing the text into a
-file.
-
-If CODING-SYSTEM is nil or omitted, the default depends on OBJECT.  If
-OBJECT is a buffer, the default for CODING-SYSTEM is whatever coding
-system would be chosen by default for writing this text into a file.
-
-If OBJECT is a string, the most preferred coding system (see the
-command `prefer-coding-system') is used.
+/* TYPE: 0 for md5, 1 for sha1. */
 
-If NOERROR is non-nil, silently assume the `raw-text' coding if the
-guesswork fails.  Normally, an error is signaled in such case.  */)
-  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror)
+Lisp_Object
+hash_function (int type, Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror, Lisp_Object binary)
 {
-  unsigned char digest[16];
-  char value[33];
   int i;
   EMACS_INT size;
   EMACS_INT size_byte = 0;
@@ -4558,6 +4533,7 @@
   register EMACS_INT b, e;
   register struct buffer *bp;
   EMACS_INT temp;
+  Lisp_Object res=Qnil;
 
   if (STRINGP (object))
     {
@@ -4728,15 +4704,92 @@
 	object = code_convert_string (object, coding_system, Qnil, 1, 0, 0);
     }
 
-  md5_buffer (SSDATA (object) + start_byte,
-	      SBYTES (object) - (size_byte - end_byte),
-	      digest);
-
-  for (i = 0; i < 16; i++)
-    sprintf (&value[2 * i], "%02x", digest[i]);
-  value[32] = '\0';
+  switch (type)
+    {
+    case 0:			/* MD5 */
+      {
+	unsigned char digest[16];
+	md5_buffer (SSDATA (object) + start_byte,
+		    SBYTES (object) - (size_byte - end_byte),
+		    digest);
+
+	if (NILP(binary))
+	  {
+	    unsigned char value[33];
+	    for (i = 0; i < 16; i++)
+	      sprintf (&value[2 * i], "%02x", digest[i]);
+	    value[32] = '\0';
+	    res = make_string (value, 32);
+	  }
+	else
+	  res = make_string (digest, 16);
+	break;
+      }
+
+    case 1:			/* SHA1 */
+      {
+	unsigned char digest[20];
+	sha1_buffer (SDATA (object) + start_byte,
+		     SBYTES (object) - (size_byte - end_byte),
+		     digest);
+	if (NILP(binary))
+	  {
+	    unsigned char value[41];
+	    for (i = 0; i < 20; i++)
+	      sprintf (&value[2 * i], "%02x", digest[i]);
+	    value[40] = '\0';
+	    res = make_string (value, 40);
+	  }
+	else
+	  res = make_string (digest, 20);
+	break;
+      }
+    }
+
+  return res;
+}
 
-  return make_string (value, 32);
+DEFUN ("md5", Fmd5, Smd5, 1, 5, 0,
+       doc: /* Return MD5 message digest of OBJECT, a buffer or string.
+
+A message digest is a cryptographic checksum of a document, and the
+algorithm to calculate it is defined in RFC 1321.
+
+The two optional arguments START and END are character positions
+specifying for which part of OBJECT the message digest should be
+computed.  If nil or omitted, the digest is computed for the whole
+OBJECT.
+
+The MD5 message digest is computed from the result of encoding the
+text in a coding system, not directly from the internal Emacs form of
+the text.  The optional fourth argument CODING-SYSTEM specifies which
+coding system to encode the text with.  It should be the same coding
+system that you used or will use when actually writing the text into a
+file.
+
+If CODING-SYSTEM is nil or omitted, the default depends on OBJECT.  If
+OBJECT is a buffer, the default for CODING-SYSTEM is whatever coding
+system would be chosen by default for writing this text into a file.
+
+If OBJECT is a string, the most preferred coding system (see the
+command `prefer-coding-system') is used.
+
+If NOERROR is non-nil, silently assume the `raw-text' coding if the
+guesswork fails.  Normally, an error is signaled in such case.  */)
+  (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object coding_system, Lisp_Object noerror)
+{
+  return hash_function (0, object, start, end, coding_system, noerror, Qnil);
+}
+
+DEFUN ("sha1", Fsha1, Ssha1, 1, 4, 0,
+       doc: /* Return the SHA1 (Secure Hash Algorithm) of an OBJECT.
+OBJECT is either a string or a buffer.
+Optional arguments BEG and END denote buffer positions for computing
+the hash of a portion of OBJECT.
+If BINARY is non-nil, return a string in binary form.  */)
+     (Lisp_Object object, Lisp_Object start, Lisp_Object end, Lisp_Object binary)
+{
+  return hash_function (1, object, start, end, Qnil, Qnil, binary);
 }
 
 \f
@@ -4911,6 +4964,7 @@
   defsubr (&Sbase64_encode_string);
   defsubr (&Sbase64_decode_string);
   defsubr (&Smd5);
+  defsubr (&Ssha1);
   defsubr (&Slocale_info);
 }
 


[-- Attachment #3: Type: text/plain, Size: 5 bytes --]


Leo

Re: Move sha1 to C?

[Chong Yidong]

Leo <sdl.web@gmail.com> writes:

> Please review the attached patch. ChangeLog entry omitted for now.
>
> Changes due to 'make sync-from-gnulib' aren't included.

It would be good to provide compability functions for `sha1-region' and
`sha1-string' as well.

Re: Move sha1 to C?

[Leo]

On 2011-05-23 23:27 +0800, Chong Yidong wrote:
> Leo <sdl.web@gmail.com> writes:
>
>> Please review the attached patch. ChangeLog entry omitted for now.
>>
>> Changes due to 'make sync-from-gnulib' aren't included.
>
> It would be good to provide compability functions for `sha1-region' and
> `sha1-string' as well.

These functions are covered by sha1 already. I think we should leave
them out. They are not used at all in the source.

Leo

Re: Move sha1 to C?

[Leo]

On 2011-05-23 15:21 +0800, Eli Zaretskii wrote:
> Note that gnulib's sha1.c uses malloc, which is not safe in Emacs.
> So either make sure sha1_stream is never ever called by Emacs, or use
> xmalloc instead.

For the record, it seems there is no need to use sha1_stream:

(defun sha1-file (file)
  (with-temp-buffer
    (set-buffer-multibyte nil)
    (insert-file-contents file)
    (sha1 (current-buffer))))

(benchmark-run 1 (sha1-file "Path-to-a-700M-file"))
(9.66299 1 0.11788399999999999)

(benchmark-run 1 (shell-command-to-string "shasum Path-to-a-700M-file"))
(5.64725 0 0.0)

Leo