thrift-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Roger Meier <ro...@bufferoverflow.ch>
Subject Re: Reporting a crash issue in t_c_glib_generator.cc
Date Thu, 28 Mar 2013 22:11:48 GMT
Could you please add a patch containing a test case and the fix here:
https://issues.apache.org/jira/browse/THRIFT-1583

thanks!
;-r

Quoting adam maxiaodong <adam.maxiaodong@gmail.com>:

> Posting here as suggested by Roger.
>
> Hello,
>
>     A crash is introduced by THRIFT-1583 c_glib leaks memory
> https://git-wip-us.apache.org/**repos/asf?p=thrift.git;a=**commit;h=**
> c75797d9060e049692c5db1617aa95**60aec939c8<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=commit;h=c75797d9060e049692c5db1617aa9560aec939c8>
>     in t_c_**glib_generator.cc line 1860 and 2529, g_free() is incorrectly
> used for type string.
>
>  @@  
> -1826,10<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l1826>
> +1828,39<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l1828>@@
>  void t_c_glib_generator::generate_object(t_struct *tstruct) {
>            }
>          }
>
> +        f_types_impl_ << indent() << "if (tobject->" << name <<
" !=
> NULL)" << endl;
> +        f_types_impl_ << indent() << "{" << endl;
> +        indent_up();
>          f_types_impl_ <<
>            indent() << destructor_function << " (tobject->" << name
<<
> -                       ", FALSE);" << endl;
> +                       ", TRUE);" << endl;
> +        f_types_impl_ << indent() << "tobject->" << name <<
" =  
> NULL;" << endl;
> +        indent_down();
> +        f_types_impl_ << indent() << "}" << endl;
>        }
> +    } else if (t->is_struct() || t->is_xception()) {
> +      string name = (*m_iter)->get_name();
> +      // TODO: g_clear_object needs glib >= 2.28
> +      // f_types_impl_ << indent() << "g_clear_object (&(tobject->"
> << name << "));" << endl;
> +      // does g_object_unref the trick?
> +      f_types_impl_ << indent() << "if (tobject->" << name <<
" !=
> NULL)" << endl;
> +      f_types_impl_ << indent() << "{" << endl;
> +      indent_up();
> +      f_types_impl_ <<
> +      indent() << "g_object_unref(tobject->" << name << ");" <<
endl;
> +      f_types_impl_ << indent() << "tobject->" << name <<
" =  
> NULL;" << endl;
> +      indent_down();
> +      f_types_impl_ << indent() << "}" << endl;
> +    } else if (t->is_string()) {
> +      string name = (*m_iter)->get_name();
> +      f_types_impl_ << indent() << "if (tobject->" << name <<
" !=
> NULL)" << endl;
> +      f_types_impl_ << indent() << "{" << endl;
> +      indent_up();
> +      f_types_impl_ <<
> +      indent() << "g_free (tobject->" << name << ");" <<
endl;
> //////////////////////////////////////////////// here
> +      f_types_impl_ << indent() << "tobject->" << name <<
" =  
> NULL;" << endl;
> +      indent_down();
> +      f_types_impl_ << indent() << "}" << endl;
>      }
>    }
>
> and
>
>
>  @@  
> -2451,12<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=d22b72324fff1134fcfeea90d5f93be1a49e7cfa;hb=d22b72324fff1134fcfeea90d5f93be1a49e7cfa#l2451>
> +2517,21<https://git-wip-us.apache.org/repos/asf?p=thrift.git;a=blob;f=compiler/cpp/src/generate/t_c_glib_generator.cc;h=b088299c8f8d8de20b35742f679989178b1e93a6;hb=b088299c8f8d8de20b35742f679989178b1e93a6#l2517>@@
>  void t_c_glib_generator::generate_deserialize_field(ofstream &out,
>    string name = prefix + tfield->get_name() + suffix;
>
>    if (type->is_struct() || type->is_xception()) {
> -    generate_deserialize_struct (out, (t_struct *) type, name, error_ret);
> +    generate_deserialize_struct (out, (t_struct *) type, name,
> error_ret, allocate);
>    } else if (type->is_container()) {
>      generate_deserialize_container (out, type, name, error_ret);
>    } else if (type->is_base_type()) {
>      t_base_type::t_base tbase = ((t_base_type *) type)->get_base();
> -
> +    if (tbase == t_base_type::TYPE_STRING) {
> +      indent(out) << "if (" << name << " != NULL)" << endl <<
> +        indent() << "{" << endl;
> +      indent_up();
> +      indent(out) << "g_free(" << name << ");" << endl <<
> //////////////////////////////////////////////// and here!
> +        indent() << name << " = NULL;" << endl;
> +      indent_down();
> +      indent(out) << "}" << endl <<
> +      endl;
> +    }
>      indent(out) << "if ((ret = thrift_protocol_read_";
>
>
>     I am wondering if anyone also hit this issue, please suggest a fix.
>
> Thanks!
> -Adam



Mime
View raw message