On Sat, Aug 11, 2018 at 11:22:09PM +0900, Kinichiro Inoguchi wrote:
> Hi,
> 
> I saw that 2 loops are almost identical, and I think diff tidy up these 2 
> loops
> in 1 function.
> 
> I found these 2 lines were not in original code.
> 
> > +           shutdown(SSL_get_fd(scon), SHUT_RDWR);
> > +           close(SSL_get_fd(scon));
> 
> Is this part needed ?

Nope, good catch.  Those are leftovers from an older
revision of this patch and would be a regression.

Updated diff below.

Index: usr.bin/openssl/s_time.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.24
diff -u -p -r1.24 s_time.c
--- usr.bin/openssl/s_time.c    13 Jul 2018 18:36:56 -0000      1.24
+++ usr.bin/openssl/s_time.c    11 Aug 2018 15:48:02 -0000
@@ -91,6 +91,7 @@ extern int verify_depth;
 
 static void s_time_usage(void);
 static SSL *doConnection(SSL * scon);
+static int benchmark(int);
 
 static SSL_CTX *tm_ctx = NULL;
 static const SSL_METHOD *s_time_meth = NULL;
@@ -244,13 +245,7 @@ tm_Time_F(int op)
 int
 s_time_main(int argc, char **argv)
 {
-       double totalTime = 0.0;
-       int nConn = 0;
-       SSL *scon = NULL;
-       time_t finishtime;
        int ret = 1;
-       char buf[1024 * 8];
-       int ver;
 
        if (single_execution) {
                if (pledge("stdio rpath inet dns", NULL) == -1) {
@@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
 
        /* Loop and time how long it takes to make connections */
 
-       bytes_read = 0;
-       finishtime = time(NULL) + s_time_config.maxtime;
-       tm_Time_F(START);
-       for (;;) {
-               if (finishtime < time(NULL))
-                       break;
-               if ((scon = doConnection(NULL)) == NULL)
-                       goto end;
-
-               if (s_time_config.www_path != NULL) {
-                       int i, retval = snprintf(buf, sizeof buf,
-                           "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
-                       if ((size_t)retval >= sizeof buf) {
-                               fprintf(stderr, "URL too long\n");
-                               goto end;
-                       }
-                       SSL_write(scon, buf, strlen(buf));
-                       while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
-                               bytes_read += i;
-               }
-               if (s_time_config.no_shutdown)
-                       SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-                           SSL_RECEIVED_SHUTDOWN);
-               else
-                       SSL_shutdown(scon);
-
-               nConn += 1;
-               if (SSL_session_reused(scon))
-                       ver = 'r';
-               else {
-                       ver = SSL_version(scon);
-                       if (ver == TLS1_VERSION)
-                               ver = 't';
-                       else if (ver == SSL3_VERSION)
-                               ver = '3';
-                       else if (ver == SSL2_VERSION)
-                               ver = '2';
-                       else
-                               ver = '*';
-               }
-               fputc(ver, stdout);
-               fflush(stdout);
-
-               SSL_free(scon);
-               scon = NULL;
-       }
-       totalTime += tm_Time_F(STOP);   /* Add the time for this iteration */
-
-       printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
read %ld\n",
-           nConn, totalTime, ((double) nConn / totalTime), bytes_read);
-       printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
-           nConn,
-           (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-           bytes_read / nConn);
+       if (benchmark(0))
+               goto end;
 
        /*
         * Now loop and time connections using the same session id over and
@@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
                goto end;
        printf("\n\nNow timing with session id reuse.\n");
 
-       /* Get an SSL object so we can reuse the session id */
-       if ((scon = doConnection(NULL)) == NULL) {
-               fprintf(stderr, "Unable to get connection\n");
+       if (benchmark(1))
                goto end;
-       }
-       if (s_time_config.www_path != NULL) {
-               int retval = snprintf(buf, sizeof buf,
-                   "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
-               if ((size_t)retval >= sizeof buf) {
-                       fprintf(stderr, "URL too long\n");
-                       goto end;
-               }
-               SSL_write(scon, buf, strlen(buf));
-               while (SSL_read(scon, buf, sizeof(buf)) > 0);
-       }
-       if (s_time_config.no_shutdown)
-               SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-                   SSL_RECEIVED_SHUTDOWN);
-       else
-               SSL_shutdown(scon);
-
-       nConn = 0;
-       totalTime = 0.0;
-
-       finishtime = time(NULL) + s_time_config.maxtime;
-
-       printf("starting\n");
-       bytes_read = 0;
-       tm_Time_F(START);
-
-       for (;;) {
-               if (finishtime < time(NULL))
-                       break;
-               if ((doConnection(scon)) == NULL)
-                       goto end;
-
-               if (s_time_config.www_path) {
-                       int i, retval = snprintf(buf, sizeof buf,
-                           "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
-                       if ((size_t)retval >= sizeof buf) {
-                               fprintf(stderr, "URL too long\n");
-                               goto end;
-                       }
-                       SSL_write(scon, buf, strlen(buf));
-                       while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
-                               bytes_read += i;
-               }
-               if (s_time_config.no_shutdown)
-                       SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-                           SSL_RECEIVED_SHUTDOWN);
-               else
-                       SSL_shutdown(scon);
-
-               nConn += 1;
-               if (SSL_session_reused(scon))
-                       ver = 'r';
-               else {
-                       ver = SSL_version(scon);
-                       if (ver == TLS1_VERSION)
-                               ver = 't';
-                       else if (ver == SSL3_VERSION)
-                               ver = '3';
-                       else if (ver == SSL2_VERSION)
-                               ver = '2';
-                       else
-                               ver = '*';
-               }
-               fputc(ver, stdout);
-               fflush(stdout);
-       }
-       totalTime += tm_Time_F(STOP);   /* Add the time for this iteration */
-
-       printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
-       printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
-           nConn,
-           (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-           bytes_read / nConn);
 
        ret = 0;
  end:
-       SSL_free(scon);
-
        if (tm_ctx != NULL) {
                SSL_CTX_free(tm_ctx);
                tm_ctx = NULL;
@@ -541,4 +407,107 @@ doConnection(SSL * scon)
                return NULL;
        }
        return serverCon;
+}
+
+static int
+benchmark(int reuse_session)
+{
+       double totalTime = 0.0;
+       int nConn = 0;
+       SSL *scon = NULL;
+       time_t finishtime;
+       int ret = 1;
+       char buf[1024 * 8];
+       int ver;
+
+       if (reuse_session) {
+               /* Get an SSL object so we can reuse the session id */
+               if ((scon = doConnection(NULL)) == NULL) {
+                       fprintf(stderr, "Unable to get connection\n");
+                       goto end;
+               }
+               if (s_time_config.www_path != NULL) {
+                       int retval = snprintf(buf, sizeof buf,
+                           "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+                       if ((size_t)retval >= sizeof buf) {
+                               fprintf(stderr, "URL too long\n");
+                               goto end;
+                       }
+                       SSL_write(scon, buf, strlen(buf));
+                       while (SSL_read(scon, buf, sizeof(buf)) > 0);
+               }
+               if (s_time_config.no_shutdown)
+                       SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+                           SSL_RECEIVED_SHUTDOWN);
+               else
+                       SSL_shutdown(scon);
+               printf("starting\n");
+       }
+
+       nConn = 0;
+       totalTime = 0.0;
+
+       finishtime = time(NULL) + s_time_config.maxtime;
+
+       bytes_read = 0;
+       tm_Time_F(START);
+
+       for (;;) {
+               if (finishtime < time(NULL))
+                       break;
+               if ((scon = doConnection(reuse_session ? scon : NULL)) == NULL)
+                       goto end;
+
+               if (s_time_config.www_path != NULL) {
+                       int i, retval = snprintf(buf, sizeof buf,
+                           "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+                       if ((size_t)retval >= sizeof buf) {
+                               fprintf(stderr, "URL too long\n");
+                               goto end;
+                       }
+                       SSL_write(scon, buf, strlen(buf));
+                       while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
+                               bytes_read += i;
+               }
+               if (s_time_config.no_shutdown)
+                       SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+                           SSL_RECEIVED_SHUTDOWN);
+               else
+                       SSL_shutdown(scon);
+
+               nConn += 1;
+               if (SSL_session_reused(scon))
+                       ver = 'r';
+               else {
+                       ver = SSL_version(scon);
+                       if (ver == TLS1_VERSION)
+                               ver = 't';
+                       else if (ver == SSL3_VERSION)
+                               ver = '3';
+                       else if (ver == SSL2_VERSION)
+                               ver = '2';
+                       else
+                               ver = '*';
+               }
+               fputc(ver, stdout);
+               fflush(stdout);
+
+               if (!reuse_session) {
+                       SSL_free(scon);
+                       scon = NULL;
+               }
+       }
+       totalTime += tm_Time_F(STOP);   /* Add the time for this iteration */
+
+       printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
read %ld\n",
+           nConn, totalTime, ((double) nConn / totalTime), bytes_read);
+       printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
+           nConn,
+           (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+           bytes_read / nConn);
+
+       ret = 0;
+ end:
+       SSL_free(scon);
+       return ret;
 }

Reply via email to