Subject:
Re: [gnupic] XWisp2 with gcc patch
From:
Daniel Serpell ####@####.####
Date:
14 Sep 2005 16:30:36 +0100
Message-Id: <20050914152959.GA4219@aplik.cl>
Hi!
El Wed, Sep 14, 2005 at 11:00:05AM +0200, Rob Hamerling escribio:
>
> I have read your patches for xwisp2, and have some comments/questions.
> Please have some patience with me, I have hardly any experience with
> using Linux and even less with programming for Linux...
No problems, thanks for doing this work!
> Daniel Serpell wrote:
>
> >xwisp2.170.linux.unwarn.patch :
> > This fixes some of the warnings on the code. This is not
> > complete, and some of the casts are really important if you
> > want xwisp2 to work under 64bit arches.
>
> If not complete, what would be missing?
I fixed some of the warnings, but there are a lot of other warnings
in the code.
The main source of warnings are using a '%lu' modiffier in a printf
statement and passing and int argument. For exameple, in file xwisp2bus.c,
function WbusHello, you define:
int rc;
End then, you use:
sprintf(szBuffer, "Communications error, rc %lu", rc);
In 64bit architectures (as AMD64), 'long' is 64bits and 'int' is 32bits,
so that won't work. To fix it, you can use '%d' in the printf (correct
for an 'int' argument), or cast the argument to 'unsigned long', or
cast the argument to 'unsigned' and use '%u' in the printf.
As I didn't know what is your prefered output format, I just added the
casts, but I think that the correct way is simply using '%d' in the
printf's.
> >+#if !defined(__LINUX__) && !defined(__linux)
>
> May I suppose that '__linux' is defined by the Linux flavour of GCC?
Yes, it's defined by GCC. On Linux, you can type "cpp -dM < /dev/null"
to get a list of predefined symbols, the most usefull are:
#define __linux 1
#define __linux__ 1
#define __unix 1
#define __unix__ 1
#define linux 1
#define unix 1
The last two are *not* defined if you compile with strict-ansi
conformance (using the -ansi switch).
> >-#include <sys\types.h>
> >-#include <sys\stat.h>
>
> The forward- in stead of back-slash works for the Open Watcom C compiler
> and for the IBM Visual Age C compiler under OS/2 too! So I can safely
> change that!
That's great!.
> >+ #define _searchenv(a,b,c) (*c=0)
>
> Is there no _searchenv() functionality in Linux?
No, there is no standard function for "path searching". In Linux, you
normaly store config files in common locations, like
"/etc/myprog/myfile.cfg", so you don't search it in the path.
For an easy fix, I simply defined the searchenv function to return an
empty string, so the rest of your code works. A better solution would be
to write a searchenv function that search's in "/etc" and your home
directory for the files. I could do that if it's usefull.
> >- sprintf(szBuffer, "WbusNext failed, rc %lu", rc);
> >+ sprintf(szBuffer, "WbusNext failed, rc %lu", (unsigned long)rc);
>
> When 'rc' is declared as 'int', wouldn't is be better to use:
> sprintf(szBuffer, "WbusNext failed, rc %d", rc);
Yes, but as that was not my code, I simply fixed it in the least
intrusive way (so the output remained the same) :-)
> >-extern int main(int argc, unsigned char **argv) {
> >+extern int main(int argc, char **argv) {
>
> I never understood the difference between a pointer to a signed or
> unsigned character, unless there are to me unknown architectures which
> have a different length for these. Never mind.....
The importance is when you actually use the pointed value, for example:
unsigned char *up = malloc(1);
char *sp = malloc(1);
int i;
*up = 0xE0;
*sp = 0xE0;
i = *up; // Here, i is 224
printf("i=%d\n",i);
i = *sp; // Here, i is -32
printf("i=%d\n",i);
Gcc uses signed chars as default, so null-terminated strings are signed
char pointers. The C standard don't specify any default, but I think
that signed chars are more common.
>
> Regards, Rob.
Thanks for your work,
Daniel
