Re: GCC4 and building gnome-vfs

[Alexander Larsson <alexl redhat com>]

On Thu, 2005-04-07 at 12:03 +0200, Kjartan Maraas wrote:
> tor, 31,.03.2005 kl. 10.26 +0200, skrev Alexander Larsson:
> > On Tue, 2005-03-29 at 14:44 +0200, Kjartan Maraas wrote:
> > > Hi.
> > > 
> > > I'm running into problems building gnome-vfs with the new GCC in Fedora
> > > Core 4 test releases. I've cooked up a patch to "fix" some of them, but
> > > I need more help with the stuff in gnome-vfs-handle.c
> > > 
> > > Attaching a patch for the stuff I have now.
> > > 
> > > The error I'm seeing that I don't see how to fix immediately is this
> > > one:
> > > 
> > > gnome-vfs-handle.c: In function '_gnome_vfs_handle_do_tell':
> > > gnome-vfs-handle.c:174: warning: pointer targets in passing argument 3
> > > of 'handle->uri->method->tell' differ in signedness
> > 
> > 
> > typedef GnomeVFSResult (* GnomeVFSMethodTellFunc)
> > 					(GnomeVFSMethod *method,
> > 					 GnomeVFSMethodHandle *method_handle,
> > 					 GnomeVFSFileOffset *offset_return);
> > 
> > vs:
> > 
> > GnomeVFSResult
> > _gnome_vfs_handle_do_tell (GnomeVFSHandle *handle,
> > 			  GnomeVFSFileSize *offset_return)
> > 
> > 
> > offset_return have different types, that indeed differ by sign.
> > 
> > GnomeVFSFileSize is whats used in the public api which can't be changed.
> > It seems right to me to.
> > 
> Failing to parse this. It seems right to you to what? I tried using
> GnomeVFSFileSize for both, but ran into other problems with that change.

It seems right to us "GnomeVFSFileSize" for the tell method instead of
"GnomeVFSFileOffset", since its never negative. Its also what the public
api uses, which is good since we then don't have to change the public
API. We need to change what type of parameter _gnome_vfs_handle_do_tell
uses though, and fix this up in all uses of it.

> > > I see from the fedora package that someone has added some cases of
> > > #undef error
> > > for some similar issues with other methods, but I'm not well enough
> > > versed in the preprocessing magic to know how this should be done in
> > > this case. Anyone?
> > 
> > The #undef calls are to handle the fact that some version of glibc had
> > macros named open etc. This is somewhat unfortunate, but ok with the
> > spec. There are nicer fixes availible though, like calling the function
> > with parenthesis around it in gnome-vfs.
> > 
> > > There are some sparse warning fixes and changes to make code static in
> > > the patch so please comment on those too if possible.
> > 
> > 
> > -                       split_service_instance (name, ptr_service, ptr_type, ptr_domain);
> > +                       split_service_instance ((unsigned char *)name, (unsigned char *)ptr_service, (unsigned char *)ptr_type, (unsigned char *)ptr_domain);
> > 
> > Sooooo ugly. Isn't there a nicer way?
> > 
> Is this better?
> 
>                 if (rr.type == ns_t_ptr) {
>                         GnomeVFSDNSSDService service;
> -                       char ptr_service[NS_MAXDNAME];
> -                       char ptr_type[NS_MAXDNAME];
> -                       char ptr_domain[NS_MAXDNAME];
> +                       unsigned char ptr_service[NS_MAXDNAME];
> +                       unsigned char ptr_type[NS_MAXDNAME];
> +                       unsigned char ptr_domain[NS_MAXDNAME];
> 
>                         len = dn_expand (reply, reply + reply_len, p, name, size of(name));
>                         if (len < 0)
>                                 goto error;
> 
> -                       split_service_instance (name, ptr_service, ptr_type, ptr _domain);
> +                       split_service_instance ((unsigned char *)name, ptr_servi ce, ptr_type, ptr_domain);
> 
> -                       if (is_valid_dns_sd_type (ptr_type)) {
> -                               service.name = g_strdup (ptr_service);
> -                               service.type = g_strdup (ptr_type);
> -                               service.domain = g_strdup (ptr_domain);
> +                       if (is_valid_dns_sd_type ((char *)ptr_type)) {
> +                               service.name = g_strdup ((char *)ptr_service);
> +                               service.type = g_strdup ((char *)ptr_type);
> +                               service.domain = g_strdup ((char *)ptr_domain);
> 
>                                 g_array_append_val(array, service);


Wouldn't it be better to make split_service_instance take char *
arguments.

> > diff -u -p -r1.2 xdgmimealias.c
> > --- libgnomevfs/xdgmimealias.c  9 Dec 2004 15:43:48 -0000       1.2
> > +++ libgnomevfs/xdgmimealias.c  29 Mar 2005 12:42:08 -0000
> > 
> > These need to go upstream too.
> > 
> >  {
> > -       FileHandle *handle = (FileHandle *)method_handle;
> > -       GnomeVFSResult res = GNOME_VFS_OK;;
> >         SmbAuthContext actx;
> >         ssize_t n = 0;
> > +       FileHandle *handle = (FileHandle *)method_handle;
> > +       GnomeVFSResult res = GNOME_VFS_OK;;
> > 
> > Does this do anything?
> > 
> Fix a case of mixed code and declarations which is not allowed any more
> it seems.

I don't get it. All the visible code is declarations. What error message
do you get?

> Suspicious double semi-colon up there too I see...

Yeah. That might be the real bug actually.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a sword-wielding Jewish filmmaker with a secret. She's a provocative 
snooty magician's assistant with her own daytime radio talk show. They fight 
crime!