List:Internals« Previous MessageNext Message »
From:Benjamin Pflugmann Date:August 30 2001 12:32am
Subject:understanding the source
View as plain text  
Hi.

While studying the source, I found some part I am not sure of
understanding. Please have a look at my change and tell whether it
would change the meaning. I think it doesn't. If it does, would you
please explain why.

If it doesn't, is there any reason it is written as it is? As IMHO
some stuff is executed needlessy, and "luckily" points to the valid
data of the last loop. Concretely, if the first line is empty or a
comment, I believe that "info.reclength=isam->s->base.reclength;"
tries to access undefined data? No harm intended, just being curious.

Even without changing the brace, I think the first "last_isam=isam;"
is not needed (btw, last_isam is undefined before). Correct? If not,
why?

Thank you in advance for any hints.

Bye,

	Benjamin.

(The diffs ignores changes in spaces to concentrate on the real change)

--- mysql-3.23.40/myisammrg/myrg_open.c	Wed Jul 18 23:19:16 2001
+++ mysql-3.23.40-philemon/myisammrg/myrg_open.c	Thu Aug 30 01:22:25 2001
@@ -61,32 +61,31 @@
   info.reclength=0;
   while ((length=my_b_gets(&file,buff,FN_REFLEN-1)))
   {
     if ((end=buff+length)[-1] == '\n')
       end[-1]='\0';
     if (buff[0] && buff[0] != '#')	/* Skipp empty lines and comments */
     {
-      last_isam=isam;
       if (!test_if_hard_path(buff))
       {
 	VOID(strmake(name_buff+dir_length,buff,
 		     sizeof(name_buff)-1-dir_length));
 	VOID(cleanup_dirname(buff,name_buff));
       }
       if (!(isam=mi_open(buff,mode,test(handle_locking))))
 	goto err;
       files++;
-    }
     last_isam=isam;
     if (info.reclength && info.reclength != isam->s->base.reclength)
     {
       my_errno=HA_ERR_WRONG_IN_RECORD;
       goto err;
     }
     info.reclength=isam->s->base.reclength;
+    }
   }
   if (!(m_info= (MYRG_INFO*) my_malloc(sizeof(MYRG_INFO)+
 				       files*sizeof(MYRG_TABLE),
 				       MYF(MY_WME))))
     goto err;
   *m_info=info;
   m_info->open_tables=(files) ? (MYRG_TABLE *) (m_info+1) : 0;
----------------------------------------------------------------------

Thread
understanding the sourceBenjamin Pflugmann30 Aug
  • bug confirmed (was: Re: understanding the source)Benjamin Pflugmann30 Aug
  • Re: understanding the sourceBenjamin Pflugmann30 Aug
    • Re: understanding the sourceMichael Widenius30 Aug
    • Re: understanding the sourceTimothy Smith30 Aug
      • Re: understanding the sourceMichael Widenius2 Sep
  • understanding the sourceMichael Widenius2 Sep