Some exec cleanups and tweaks for photo ID and keyserver execution

g10/ChangeLog

@@ -1,3 +1,20 @@
+2001-12-27  David Shaw  <dshaw@jabberwocky.com>
+
+	* exec.c (exec_finish): Show errors when temp files cannot be
+	deleted for whatever reason.
+
+	* exec.c (exec_read): Don't rely on WEXITSTATUS being present.
+
+	* exec.c (make_tempdir): Add temp file creator for win32.  Don't
+	create an incoming temp file if the exec is write-only.
+
+	* keyserver.c (keyserver_spawn): Clean up error handling, for when
+	the spawn fails.
+
+	* photoid.c (show_photo): Clean up error handling.
+
+	* misc.c (check_permissions): Neaten.
+
 2001-12-25  David Shaw  <dshaw@jabberwocky.com>
 
 	* mkdtemp.c (mkdtemp): Add copyleft info and tweak the 'X' counter

g10/exec.c

@@ -24,7 +24,7 @@
 #include <stdio.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#ifndef HAVE_DOSISH_SYSTEM
+#ifndef EXEC_TEMPFILE_ONLY
 #include <sys/wait.h>
 #endif
 #include <fcntl.h>
@@ -58,9 +58,21 @@ static int make_tempdir(struct exec_info *info)
 	    {
 #ifdef __riscos__
 	      tmp="<Wimp$ScrapDir>";
-#elsif HAVE_DOSISH_SYSTEM
+#elif defined (__MINGW32__) || defined (__CYGWIN32__)
-	      tmp=m_alloc(1024);
+	      tmp=m_alloc(256);
-	      GetTempPath(1023,tmp);
+	      if(GetTempPath(256,tmp)==0)
+		strcpy(tmp,"c:\temp");
+	      else
+		{
+		  int len=strlen(tmp);
+
+		  /* GetTempPath may return with \ on the end */
+		  while(len>0 && tmp[len-1]=='\\')
+		    {
+		      tmp[len-1]='\0';
+		      len--;
+		    }
+		}
 #else
 	      tmp="/tmp";
 #endif
@@ -72,7 +84,7 @@ static int make_tempdir(struct exec_info *info)
 
   sprintf(info->tempdir,"%s" DIRSEP_S "gpg-XXXXXX",tmp);
 
-#ifdef HAVE_DOSISH_SYSTEM
+#if defined (__MINGW32__) || defined (__CYGWIN32__)
   m_free(tmp);
 #endif
 
@@ -87,16 +99,19 @@ static int make_tempdir(struct exec_info *info)
       sprintf(info->tempfile_in,"%s" DIRSEP_S "datain" EXTSEP_S "%s",
 	      info->tempdir,info->binary?"bin":"txt");
 
-      info->tempfile_out=m_alloc(strlen(info->tempdir)+1+11+1);
+      if(!info->writeonly)
-      sprintf(info->tempfile_out,"%s" DIRSEP_S "dataout" EXTSEP_S "%s",
+	{
-	      info->tempdir,info->binary?"bin":"txt");
+	  info->tempfile_out=m_alloc(strlen(info->tempdir)+1+11+1);
+	  sprintf(info->tempfile_out,"%s" DIRSEP_S "dataout" EXTSEP_S "%s",
+		  info->tempdir,info->binary?"bin":"txt");
+	}
     }
 
   return info->madedir?0:G10ERR_GENERAL;
 }
 
-/* Expands %i and %o in the args to the full temp files (within the
+/* Expands %i and %o in the args to the full temp files within the
-   temp directory */
+   temp directory. */
 static int expand_args(struct exec_info *info,const char *args_in)
 {
   const char *ch=args_in;
@@ -193,10 +208,8 @@ static int expand_args(struct exec_info *info,const char *args_in)
 }
 
 /* Either handles the tempfile creation, or the fork/exec.  If it
-   returns ok, then info->tochild is a FILE * that can be written
+   returns ok, then info->tochild is a FILE * that can be written to.
-   to. */
+   The rules are: if there are no args, then it's a fork/exec/pipe.
-
-/* The rules are: if there are no args, then it's a fork/exec/pipe.
    If there are args, but no tempfiles, then it's a fork/exec/pipe via
    shell -c.  If there are tempfiles, then it's a system. */
 
@@ -355,10 +368,9 @@ int exec_write(struct exec_info **info,const char *program,
       goto fail;
     }
 
-  return 0;
+  ret=0;
 
  fail:
-  exec_finish(*info);
   return ret;
 }
 
@@ -379,12 +391,21 @@ int exec_read(struct exec_info *info)
 	{
 	  log_error(_("system error while calling external program: %s\n"),
 		    strerror(errno));
+	  info->progreturn=127;
 	  goto fail;
 	}
+      else
+	{
+#ifdef WEXITSTATUS
+	  info->progreturn=WEXITSTATUS(info->progreturn);
+#else
+	  info->progreturn/=256;
+#endif
+	}
 
-    #ifndef HAVE_DOSISH_SYSTEM
+      /* 127 is the magic value returned from system() to indicate
-      info->progreturn=WEXITSTATUS(info->progreturn);
+         that the shell could not be executed, or from /bin/sh to
-    #endif      
+         indicate that the program could not be executed. */
 
       if(info->progreturn==127)
 	{
@@ -405,10 +426,9 @@ int exec_read(struct exec_info *info)
 	}
     }
 
-  return 0;
+  ret=0;
 
  fail:
-  exec_finish(info);
   return ret;
 }
 
@@ -436,12 +456,22 @@ int exec_finish(struct exec_info *info)
   if(info->madedir && !info->keep_temp_files)
     {
       if(info->tempfile_in)
-	unlink(info->tempfile_in);
+	{
+	  if(unlink(info->tempfile_in)==-1)
+	    log_info(_("Warning: unable to remove tempfile (%s) \"%s\": %s\n"),
+		     "in",info->tempfile_in,strerror(errno));
+	}
   
       if(info->tempfile_out)
-	unlink(info->tempfile_out);
+	{
+	  if(unlink(info->tempfile_out)==-1)
+	    log_info(_("Warning: unable to remove tempfile (%s) \"%s\": %s\n"),
+		     "out",info->tempfile_out,strerror(errno));
+	}
 
-      rmdir(info->tempdir);
+      if(rmdir(info->tempdir)==-1)
+	log_info(_("Warning: unable to remove temp directory \"%s\": %s\n"),
+		 info->tempdir,strerror(errno));
     }
 
   m_free(info->command);

g10/keyserver.c

@@ -452,7 +452,7 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count,int *prog)
 
   if(!gotversion)
     {
-      log_error(_("keyserver communications error\n"));
+      log_error(_("keyserver did not send VERSION\n"));
       goto fail;
     }
 
@@ -510,9 +510,8 @@ keyserver_spawn(int action,STRLIST list,u32 (*kidlist)[2],int count,int *prog)
       break;
     }
 
-  *prog=exec_finish(spawn);
-
  fail:
+  *prog=exec_finish(spawn);
 
   return ret;
 }
@@ -558,13 +557,11 @@ keyserver_work(int action,STRLIST list,u32 (*kidlist)[2],int count)
 
   /* It's not the internal HKP code, so try and spawn a handler for it */
 
-  if((rc=keyserver_spawn(action,list,kidlist,count,&ret))==0)
+  rc=keyserver_spawn(action,list,kidlist,count,&ret);
+  if(ret)
     {
       switch(ret)
 	{
-	case KEYSERVER_OK:
-	  break;
-
 	case KEYSERVER_SCHEME_NOT_FOUND:
 	  log_error(_("no handler for keyserver scheme \"%s\"\n"),
 		    opt.keyserver_scheme);
@@ -576,12 +573,12 @@ keyserver_work(int action,STRLIST list,u32 (*kidlist)[2],int count)
 	  break;
 	}
 
-      /* This is not the best error code for this */
+      return G10ERR_KEYSERVER;
-      return G10ERR_INVALID_URI;
     }
-  else
+
+  if(rc)
     {
-      log_error(_("keyserver communications error\n"));
+      log_error(_("keyserver communications error: %s\n"),g10_errstr(rc));
 
       return rc;
     }

g10/misc.c

@@ -341,8 +341,8 @@ openpgp_md_test_algo( int algo )
 int
 check_permissions(const char *path,int checkonly)
 {
-#ifndef HAVE_DOSISH_SYSTEM
+#if defined(HAVE_STAT) && !defined(HAVE_DOSISH_SYSTEM)
-#ifdef HAVE_STAT
+
   struct stat statbuf;
   int isdir=0;
 
@@ -392,8 +392,8 @@ check_permissions(const char *path,int checkonly)
 		 isdir?"directory":"file",path);
       return 1;
     }
-#endif
+
-#endif /*!HAVE_DOSISH_SYSTEM*/
+#endif /* HAVE_STAT && !HAVE_DOSISH_SYSTEM */
 
   return 0;
 }

g10/photoid.c

@@ -236,12 +236,18 @@ void show_photo(const struct user_attribute *attr,PKT_public_key *pk)
   command[PHOTO_COMMAND_MAXLEN-1]='\0';
 
   if(exec_write(&spawn,NULL,command,1,1)!=0)
-    goto fail;
+    {
+      exec_finish(spawn);
+      goto fail;
+    }
 
   fwrite(attr->data,attr->len,1,spawn->tochild);
 
   if(exec_read(spawn)!=0)
-    goto fail;
+    {
+      exec_finish(spawn);
+      goto fail;
+    }
 
   if(exec_finish(spawn)!=0)
     goto fail;

include/ChangeLog

@@ -1,3 +1,10 @@
+2001-12-27  David Shaw  <dshaw@jabberwocky.com>
+
+	* KEYSERVER_SCHEME_NOT_FOUND should be 127 to match the POSIX
+	system() (via /bin/sh) way of signaling this.
+	
+	* Added G10ERR_KEYSERVER
+
 2001-12-27  Werner Koch  <wk@gnupg.org>
 
 	* util.h [MINGW32]: Fixed name of include file.

include/errors.h

@@ -74,6 +74,7 @@
 #define G10ERR_NOT_PROCESSED  52
 #define G10ERR_UNU_PUBKEY     53
 #define G10ERR_UNU_SECKEY     54
+#define G10ERR_KEYSERVER      55
 
 
 #ifndef HAVE_STRERROR

include/keyserver.h

@@ -23,8 +23,8 @@
 
 /* Return codes */
 #define KEYSERVER_OK               0
-#define KEYSERVER_SCHEME_NOT_FOUND 1
+#define KEYSERVER_INTERNAL_ERROR   1
-#define KEYSERVER_INTERNAL_ERROR   2
+#define KEYSERVER_VERSION_ERROR    2
-#define KEYSERVER_VERSION_ERROR    3
+#define KEYSERVER_SCHEME_NOT_FOUND 127
 
 #endif /* !_KEYSERVER_H_ */

util/ChangeLog

@@ -1,3 +1,7 @@
+2001-12-27  David Shaw  <dshaw@jabberwocky.com>
+
+	* errors.c (g10_errstr): Added G10ERR_KEYSERVER
+
 2001-12-27  Werner Koch  <wk@gnupg.org>
 
 	* simple-gettext.c [MINGW32]: Fixed last changed.

util/errors.c

@@ -104,6 +104,7 @@ g10_errstr( int err )
       /* the key cannot be used for a specific usage */
       X(UNU_PUBKEY     ,N_("unusable public key"))
       X(UNU_SECKEY     ,N_("unusable secret key"))
+      X(KEYSERVER      ,N_("keyserver error"))
       default: p = buf; sprintf(buf, "g10err=%d", err); break;
     }
   #undef X