Re: core segfaults (Yahoo?)

On Thursday 11 March 2004 05:38, A. Craig West wrote:
> On Wed, 10 Mar 2004, A. Craig West wrote:
> ===================================================================
> --- core/src/plugin.c	(revision 306)
> +++ core/src/plugin.c	(working copy)
> @@ -121,6 +121,18 @@
>  }
>
>  #if HAVE_LIBLTDL  // MOTHBALLED
> +static eb_plugin * create_plugin(char * fnam)
> +{
> +  eb_plugin * plugin = (eb_plugin *)malloc(sizeof(eb_plugin));
> +  plugin->filename=(char *)strdup(fnam);
> +  plugin->info=NULL;
Note this line.
> +  plugin->status=PLUGIN_NOT_LOADED;
> +  plugin->errormsg=NULL;
> +  plugins=e_list_append(plugins, plugin);
> +
> +  return plugin;
> +}
> +
>  static void add_error_entry(char * fnam, char * errormsg)
>  {
>    eb_plugin * plugin;
> @@ -128,20 +140,23 @@
>
>    if((plugin=plugin_by_name(fnam))==NULL)
>    {
> -    plugin=(eb_plugin *)malloc(sizeof(eb_plugin));
> +    plugin=create_plugin(fnam);
>      isnew=1;
>    }
>
> -  plugin->filename=fnam;
> +  free(plugin->info);
> +  free(plugin->errormsg);
...but then, if there's an error, we create the plugin with NULL info 
and errormsg, and then free them straight away. Huh?

>    plugin->info=NULL;
>    plugin->status=PLUGIN_CANNOT_LOAD;
>    plugin->errormsg=(char *)strdup(errormsg);
Now that looks better...

>    if(isnew)
>    {
> -    plugins=e_list_append(plugins, plugin);
Why aren't we adding it any more here?
>      new_plugin_notify(plugin);
> -  } else {
> +  }
> +  else
> +  {
Personally, I prefer
} else {
rather than
}
else
{

...your call, in your code, but please don't go changing my formatting 
on my turf on too regular a basis.

>      plugin_update_notify(plugin);
>    }
>  }
> @@ -154,6 +169,7 @@
>    dlhandle dat;
>    eb_plugin_info * info;
>    eb_plugin * plugin;
> +  int isnew=0;
>
>    full_path=(char *)malloc(strlen(path)+strlen(fnam)+2);
>    sprintf(full_path, "%s/%s", path, fnam);
> @@ -192,12 +208,27 @@
>    // Now, we dispatch based on plugin type. Are we all sitting
> comfortably?
>
>    if(plugin==NULL)
> -  { plugin=(eb_plugin *)malloc(sizeof(eb_plugin)); }
> +  {
> +    plugin=create_plugin(full_path);
> +    isnew=1;
> +  }
>
> +  free(plugin->info);
> +  free(plugin->errormsg);
Once again, we free(NULL) if plugin==NULL...

>    plugin->status=PLUGIN_LOADED;
>    plugin->errormsg=NULL;
>    plugin->info=info;
>
> +  if(isnew)
> +  {
> +    new_plugin_notify(plugin);
> +  }
> +  else
> +  {
> +    plugin_update_notify(plugin);
> +  }
OK, I can believe that this is necessary (the plugin notify API is 
pretty vestigial right now, anyhow).


Meredydd