[question] 不要な変数?など

[blacktoad30]

src/gimv_thumb_view.cにて、気になる箇所があります。

  2026 | void
  2027 | gimv_thumb_view_open_image (GimvThumbView *tv, GimvThumb *thumb, gint type)
  2028 | {
  2029 |    GimvThumbWin *tw;
  2030 |    GimvImageWin *iw = NULL;
  2031 |    GimvImageView   *iv = NULL;
  2032 |    GList *current, *node;
  2033 |    const gchar *image_name, *ext;
  2034 |    gchar *filename, *tmpstr;
  2035 | 
  2036 |    g_return_if_fail (GIMV_IS_THUMB_VIEW (tv) && thumb);
  2037 | 
  2038 |    tw = tv->tw;
  2039 |    g_return_if_fail (GIMV_IS_THUMB_WIN (tw));
  2040 | 
  2041 |    image_name = gimv_image_info_get_path (thumb->info);
  2042 |    g_return_if_fail (image_name && *image_name);
  2043 |    filename = g_strdup (image_name);
  2044 | 
  2045 |    gchar *basename = g_path_get_basename (filename);
  2046 |    if (!strcmp ("..", basename)) {
  2047 |       tmpstr = filename;
  2048 |       filename = g_dirname (filename);
  2049 |       g_free (tmpstr);
  2050 |       if (filename) {
  2051 |          tmpstr = filename;
  2052 |          filename = g_dirname (filename);
  2053 |          g_free (tmpstr);
  2054 |       }
  2055 |       tmpstr = NULL;
  2056 |    }

まず、2046~2056行目のif文の中で、なぜtmpstrが必要なのかが分かりません。filenameのポインタを代入した後は特に利用することもなくg_free()されているので、これは削除しても良いと思うのですが、いかがでしょうか。

次に、入れ子の中のif文で、2回目のfilename = g_dirname (filename)をしているのも気になりました。条件をANDで一纏めにできそうです。

glibの仕様を把握できていないか、勘違いだったらすみません。

[ashie]

まず、2046~2056行目のif文の中で、なぜtmpstrが必要なのかが分かりません。filenameのポインタを代入した後は特に利用することもなくg_free()されているので、これは削除しても良いと思うのですが、いかがでしょうか。

分かりにくくて申し訳ありませんが、単に2048行目のfilename = g_dirname (filename);の前後のコードを削除するだけだとメモリリークします。

ここのコードは、最初のfilenameが例えば/path/to/some/directory/..というディレクトリ名だったときに、..が指し示す先の/path/to/someの部分だけを取り出すという処理です。一回目で..を削って/path/to/some/directoryにして、2回目でdirectoryを削って/path/to/someにしています。

• 最初のfilenameは/path/to/some/directory/..
• g_dirname()は引数で与えたパスからディレクトリ名だけを取り出す関数ですが、このときに新しいメモリ領域を確保します
• 一回目のg_dirname()で/path/to/some/directoryを新規メモリに確保してそのポインタを再度filenameに格納します
• 古い方の/path/to/some/directory/..の方は不要になったのでg_free()で解放したいのですが、filenameは既に新しい方の/path/to/some/directoryを指し示していますので、g_free(filename)で解放できません。
• g_dirname()実行後に古い方の/path/to/some/directory/..を解放できるように、一時変数のtmpstrに待避させています。
• 2回目も同様です（/path/to/some/directoryを/path/to/someに縮める）

filenameを使い回していることや、ディレクトリ名なのにfilenameと言ってることがわかりにくさの一因かなと思います。例えば以下のようにした方が良いのかも

gchar *dirname = g_dirname (filename);
g_free(filename);
// 以後はdirnameを使う

[blacktoad30]

返信ありがとうございます。このif文の処理の内容は把握できました。filenameという名前に引きずられて混乱していました...
変数の名前の変更など、改善の余地がありそうなので検討してみます。