2

我之前已经在Code Review中实施了一些建议。我还通过使用指针改进了我的代码。但是，下面的地址递增部分有什么问题squeezed_str++？似乎地址没有增加。请指教。

PS。substring() 函数正在工作。:)

char *squeeze (char *str, int start_index, int end_index, char *ref_str) {
     char *substr;
     substr = malloc (sizeof (*substr));
     if (substr == NULL) {
          printf ("Unable to allocate memory.\n");
          exit (EXIT_FAILURE);
     }

     char *squeezed_str;
     squeezed_str = malloc (sizeof (*squeezed_str));
     if (squeezed_str == NULL) {
          printf ("Unable to allocate memory!\n");
          exit (EXIT_FAILURE);
     }

     substr = substring (str, start_index, end_index);
     int substr_len = strlen (substr);
     int refstr_len = strlen (ref_str);

     char chr1, chr2; chr1 = chr2 = '\0';

     for (int i = 0; i < substr_len; i++) {
          chr1 = *(substr+i);
          for (int j = 0; j < refstr_len; j++) {
               chr2 = *(ref_str + j);
               if (chr1 == chr2) {
                    break;
               }
          }
          if (chr1 != chr2) {
               *squeezed_str = *(substr+i);
               squeezed_str++;
          }     
     }

     return squeezed_str;
 } /* end of squeeze() */
4

2 回答 2

2

内联建议的一些改进，未经测试：

char *squeeze (char *str, int start_index, int end_index, char *ref_str) {
     char *substr = substring (str, start_index, end_index);
         //do not malloc here!, you are doing an assignment later on, so memory leak
         //infact might as well move assignment right here

     char *squeezed_str;
     squeezed_str = malloc (strlen(str)+1); //+ 1 one for null terminator
     if (squeezed_str == NULL) {
          printf ("Unable to allocate memory!\n");
          exit (EXIT_FAILURE);
     }

     int substr_len = strlen (substr);
     int refstr_len = strlen (ref_str);
     int squeezeStrIdx = 0;

     for (int i = 0; i < substr_len; i++) {
          char chr1 = substr[i]; //using index accessing is nicer
                                 //moved the scope of the variable in
          char chr2 = '\0'; //reduced variable scope, benefit is now resets each loop
          for (int j = 0; j < refstr_len; j++) {
               chr2 = ref_str[j];
               if (chr1 == chr2) {
                    break;
               }
          }
          if (chr1 != chr2) {
               squeezed_str[squeezeStrIdx] = chr1;
               squeezeStrIdx++; //if you modify squeezed_str,
                                //how do you expect to return it at end of method?
          }     
     }

     //this is to null terminate the squeezed_str
     squeezed_str[squeezeStrIdx] = '\0';

     return squeezed_str;
 } /* end of squeeze() */
于 2012-11-03T09:16:28.833 回答
1

完全有缺陷的一件事是完全修改squeezed_str。这是返回的地址malloc。如果您修改它，您以后将无法访问free该内存区域。您造成了内存泄漏，或者更糟糕的是，您free在最后调用时造成了崩溃。

编辑：

 char *squeeze (const char *str, size_t start_index, size_t end_index, const char *ref_str) 
 /* You don't modify `str` and `ref_str` so it is a good idea to make it visible in the signature of the function, whence the const qualifiers */
 {
 char *substr;   
 /* Snip the memory leak */

 char *squeezed_str = malloc (sizeof (*squeezed_str));
 if (!squeezed_str) {
      fprintf (stderr, "Unable to allocate memory!\n");  /* Error are to be written on stderr */
      exit (EXIT_FAILURE);     /* Exitting the app in a function is not good style */
 }

 substr = substring (str, start_index, end_index);
 size_t substr_len = strlen (substr);       /* strlen returns size_t not int */
 size_t refstr_len = strlen (ref_str);

 char chr1 = 0, chr2 = 0;
 char *squeezed_copy = squeezed_str;       /* We use a second pointer to not lose the malloc'ed area */
 for (size_t i = 0; i < substr_len; i++) {
      chr1 = substr[i];                     /* This syntax is simpler (you'll see when you have nested structures */
      for (size_t j = 0; j < refstr_len; j++) {
           chr2 = ref_str[j];
           if (chr1 == chr2) {
                break;
           }
      }
      if (chr1 != chr2)
           *squeezed_copy++ = substr[i];
 }
 /* Add the NUL sentinel */
 *squeezed_copy++ = 0;

 return squeezed_str;
} /* end of squeeze() */

这就是说，我还没有检查算法是否符合您的预期。预感我会说使用strstr会是一个更好的主意。

于 2012-11-03T09:17:46.130 回答