Re: PATCH: type errors in src-tree
On Mon, Mar 03, 2003 at 12:08:04AM +0100, Jens Rehsack wrote: Now, that OpenWatcom is released, the FreeBSd port of it should follow. And maybe someone will try to compile the kernel and world with it. I hate to be the skeptic, but looking at OpenWatcom 1.0, it only produces dos and win32 binaries. It will be a *long* time until it targets Unix correctly. If that would work, this would be great, because the watcom compiler generates much better code than gcc does, even than gcc -O3 (and all known optimizations on). Rather than just repeat some old wife's tale; can anyone produce a real analysis backing this statement up? To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
From: David O'Brien [EMAIL PROTECTED] To: Jens Rehsack [EMAIL PROTECTED] Cc: [EMAIL PROTECTED] Sent: Wednesday, March 05, 2003 10:01 AM Subject: Re: PATCH: type errors in src-tree On Mon, Mar 03, 2003 at 12:08:04AM +0100, Jens Rehsack wrote: Now, that OpenWatcom is released, the FreeBSd port of it should follow. And maybe someone will try to compile the kernel and world with it. I hate to be the skeptic, but looking at OpenWatcom 1.0, it only produces dos and win32 binaries. It will be a *long* time until it targets Unix correctly. Just FYI: Well, not only dos and win32, but it will be really long... What they have now: From: Bart Oldeman [EMAIL PROTECTED] Newsgroups: openwatcom.contributors Sent: Sunday, March 02, 2003 7:36 AM Subject: Re: bootstrap on linux In article [EMAIL PROTECTED], Andi Kleen wrote: Michal Necasek [EMAIL PROTECTED] writes: Andi Kleen wrote: does a native bootstrap on linux of openwatcom work yet? No. Some of the tools can be built and some even work but not enough to get the build environment going. Can you elaborate a bit on it. What does work, what doesn't? the compilers work (wcc386 and wcc), compiled using gcc and compiled using watcom. they can however (still) only reliably output OMF objects. wlink can be cross-compiled for Linux but crashes (SIGSEGV) if you run it there to combine several OMFs into an ELF executable. Without a working linker the Linux hosted compiler isn't very useful yet -- and a full bootstrap impossible. There has been an attempt to cross-compile wasm, I'm not sure how far that went. wmake cannot be compiled yet -- it uses spawnxx calls that would need to be translated into fork()s and execve()s for Linux (using a wrapper or to be implemented in the Watcom LIBC). And Linux development has stalled for the last month (lack of time of the contributors). Bart heh If that would work, this would be great, because the watcom compiler generates much better code than gcc does, even than gcc -O3 (and all known optimizations on). Rather than just repeat some old wife's tale; can anyone produce a real analysis backing this statement up? Not me :) Igor To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
On Tue, Mar 04, 2003 at 11:01:25PM -0800, David O'Brien wrote: On Mon, Mar 03, 2003 at 12:08:04AM +0100, Jens Rehsack wrote: Now, that OpenWatcom is released, the FreeBSd port of it should follow. And maybe someone will try to compile the kernel and world with it. I hate to be the skeptic, but looking at OpenWatcom 1.0, it only produces dos and win32 binaries. It will be a *long* time until it targets Unix correctly. I think the bigger problem is to make the compiler a Unix program itself. The whole tree is pretty much non-portable at this time. Generating code for Unix is the least of the problems (assuming no compiler special libc implementation). I started playing with TenDRA as well and even though the compiler isn't usable yet, it's much more Unix oriented. Instead of mucking with getting a proprietary make variant to compile or figuring out if you should replace all slash options for dash options, you can pretty much focus on the compiler itself from the word go. Needless to say that for OW I'm going to piggyback the Linux port. TenDRA has a higher chance of being able to compile world and kernel I think before OW. -- Marcel Moolenaar USPA: A-39004 [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
On Sun, 2 Mar 2003, Jens Rehsack wrote: JRBarney Wolff wrote: JR This is an example of what I was pointing out: JR JR On Sun, Mar 02, 2003 at 01:53:33AM +0100, Jens Rehsack wrote: JR ... JR JR@@ -1444,22 +1420,19 @@ JR *none- response sent JR * JR */ JR-void JR-send_resp ( intf, Hdr, resp ) JR- int intf; JR- Snmp_Header *Hdr; JR- u_char *resp; JR+static void JR+send_resp ( const int intf, Snmp_Header *Hdr, char *resp ) JR { JR int n; JR JR- if ( ilmi_fd[intf] 0 ) { JR- n = write ( ilmi_fd[intf], (caddr_t)resp[1], resp[0] ); JR+ if ( ilmi_fd[intf] 0 ) { /* FIXME: does ilmi_fd[intf] exists? out of range? */ JR+ n = write ( ilmi_fd[intf], resp+1, resp[0] ); JR JR ... JR JR Here's a case where it matters whether something is u_char or char. JR write(2) takes a size_t as its third arg, and extension of a char to JR that may not be the same as for u_char, for example on Sparc. If the JR response is ever 127 bytes, this will fail. You're going to have to JR look carefully at how things are used to see when char is appropriate JR and when u_char is necessary. JR JR JRThat is really right, but for those check I have to know more 'bout ATM, JRright? I just have detected some compiler errors using JR-finline-functions (yes, I'm playing with optimization options :-)). If JRyou know a real good online-reference, one fine day I'll check it and JRcheck the entire ilmid.c code for valid signment. Go to www.atmforum.com and look at the Standards page. You will find the ILMI 4.0 standard there. But beware, if you are not used to read telecommunication standards, you will be confused :-) For ILMI you will also need the relavant SNMP RFCs and, maybe, the ASN.1 doc (don't remember exactly should be one of the Z.* ITU-T standards). harti JR JRJens JR JR JRTo Unsubscribe: send mail to [EMAIL PROTECTED] JRwith unsubscribe freebsd-current in the body of the message JR -- harti brandt, http://www.fokus.gmd.de/research/cc/cats/employees/hartmut.brandt/private [EMAIL PROTECTED], [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
John Polstra writes: In article [EMAIL PROTECTED], Dag-Erling Smorgrav [EMAIL PROTECTED] wrote: This is wrong. caddr_t should be uniersally replaced with void *. Not quite. There is (or at least used to be) a lot of code that assumed you could do address arithmetic on a caddr_t. You can't do that on a void *, at least not in ANSI C. I think gcc lets you do it, but it's an extension. As I have discovered. I specifically looked for this, and my misreading of the spec is now clear. :-) M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Mark Murray wrote: John Polstra writes: In article [EMAIL PROTECTED], Dag-Erling Smorgrav [EMAIL PROTECTED] wrote: This is wrong. caddr_t should be uniersally replaced with void *. Not quite. There is (or at least used to be) a lot of code that assumed you could do address arithmetic on a caddr_t. You can't do that on a void *, at least not in ANSI C. I think gcc lets you do it, but it's an extension. As I have discovered. I specifically looked for this, and my misreading of the spec is now clear. :-) Yes, but relying on this during fixing out caddr_t may break use of other compilers. Now, that OpenWatcom is released, the FreeBSd port of it should follow. And maybe someone will try to compile the kernel and world with it. If that would work, this would be great, because the watcom compiler generates much better code than gcc does, even than gcc -O3 (and all known optimizations on). So long, Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
On Sunday 02 March 2003 18:08, Jens Rehsack wrote: Now, that OpenWatcom is released, the FreeBSd port of it should follow. And maybe someone will try to compile the kernel and world with it. If that would work, this would be great, because the watcom compiler generates much better code than gcc does, even than gcc -O3 (and all known optimizations on). Is someone working on such a port? Will a bootstrap port to build it with GCC be part of the work? -- Chris BeHanna http://www.pennasoft.com Principal Consultant PennaSoft Corporation [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Jens Rehsack [EMAIL PROTECTED] writes: Of course. Very often in ilmid.c the type caddr_t was used, and nearly the same count of 'const char *'s was used. I've searched the include files for caddr_t (core address) and found it defined as 'char *', so I decided to used commonly caddr_t - maybe later I check which of them could be changed into 'c_caddr_t' for being const. But You can of couse replace all 'caddr_t' which 'char *'. This is wrong. caddr_t should be uniersally replaced with void *. DES -- Dag-Erling Smorgrav - [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Dag-Erling Smorgrav wrote: Jens Rehsack [EMAIL PROTECTED] writes: Of course. Very often in ilmid.c the type caddr_t was used, and nearly the same count of 'const char *'s was used. I've searched the include files for caddr_t (core address) and found it defined as 'char *', so I decided to used commonly caddr_t - maybe later I check which of them could be changed into 'c_caddr_t' for being const. But You can of couse replace all 'caddr_t' which 'char *'. This is wrong. caddr_t should be uniersally replaced with void *. Good to know. I think I have done it where it's possible, and where really (unsigned) char *(*) was required, I've used that. There're some places in code where I'm not sure about it's being correct, but that has nothing to do with char */void *. DES Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
In article [EMAIL PROTECTED], Dag-Erling Smorgrav [EMAIL PROTECTED] wrote: This is wrong. caddr_t should be uniersally replaced with void *. Not quite. There is (or at least used to be) a lot of code that assumed you could do address arithmetic on a caddr_t. You can't do that on a void *, at least not in ANSI C. I think gcc lets you do it, but it's an extension. John -- John Polstra John D. Polstra Co., Inc.Seattle, Washington USA Disappointment is a good sign of basic intelligence. -- Chögyam Trungpa To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
John Polstra [EMAIL PROTECTED] writes: Dag-Erling Smorgrav [EMAIL PROTECTED] wrote: This is wrong. caddr_t should be uniersally replaced with void *. Not quite. There is (or at least used to be) a lot of code that assumed you could do address arithmetic on a caddr_t. You can't do that on a void *, at least not in ANSI C. I think gcc lets you do it, but it's an extension. Correct, and it will break if compiled with the options we use to build kernels, but in the great majority of cases, caddr_t can be replaced with void *. DES -- Dag-Erling Smorgrav - [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Sorry for resending (3rd time), but I've found a small typo in the patch of sbin/atm/ilmid/ilmid.c Jens --- sbin/atm/ilmid/ilmid.c.orig Wed Jan 1 18:48:45 2003 +++ sbin/atm/ilmid/ilmid.c Sat Mar 1 13:29:05 2003 @@ -162,7 +162,7 @@ union { int ival; /* INTEGER/TIMESTAMP */ Objid oval; /* OBJID */ - longaval; /* IPADDR */ + uint32_taval; /* IPADDR */ charsval[STRLEN]; /* OCTET */ } var; Variable*next; @@ -173,10 +173,10 @@ * which doesn't have the last three fields is the TRAP type. */ struct snmp_header { - int pdulen; - int version; + uint32_tpdulen; + uint32_tversion; charcommunity[64]; - int pdutype; + uint32_tpdutype; /* GET/GETNEXT/GETRESP/SET */ int reqid; @@ -185,11 +185,11 @@ /* TRAP */ Objid enterprise; - int ipaddr; + uint32_tipaddr; int generic_trap; int specific_trap; - int varlen; + uint32_tvarlen; Variable*head, *tail; }; @@ -279,7 +279,7 @@ * Temporary buffer for building response packets. Should help ensure * that we aren't accidently overwriting some other memory. */ -u_char Resp_Buf[1024]; +char Resp_Buf[1024]; /* * Copy the reponse into a buffer we can modify without @@ -292,7 +292,7 @@ * TRAP generic trap types */ char *Traps[] = { coldStart, warmStart, linkDown, linkUp, - authenticationFailure, egpNeighborLoss, + authenticationFailure, egpNeighborLoss, enterpriseSpecific }; @@ -320,6 +320,9 @@ */ Objid addressEntry[MAX_UNITS + 1]; +static const char *ilmi_ident_str = ILMI; +static const size_tilmi_ident_str_len = strlen(ILMI); + /* * When this daemon started */ @@ -335,7 +338,7 @@ #defineLOG_FILE/var/log/ilmid FILE *Log; /* File descriptor for log messages */ -void set_reqid( u_char *, int ); +void set_reqid( caddr_t, uint32_t ); void Increment_DL( int ); void Decrement_DL( int ); @@ -376,7 +379,7 @@ /* * Utility to pretty print buffer as hex dumps - * + * * Arguments: * bp - buffer pointer * len - length to pretty print @@ -387,10 +390,10 @@ */ void hexdump ( bp, len ) - u_char *bp; - int len; + caddr_t bp; + uint32_tlen; { - int i, j; + uint32_ti, j; /* * Print as 4 groups of four bytes. Each byte is separated @@ -443,7 +446,7 @@ * bufp- pointer to buffer pointer * plen- pointer to PDU length or NULL if not a concern * - * Returns: + * Returns: * bufp- updated buffer pointer * plen- (possibly) adjusted pdu length * len - decoded length @@ -451,21 +454,21 @@ */ int asn_get_pdu_len ( bufp, plen ) - u_char **bufp; - int *plen; + caddr_t *bufp; + uint32_t*plen; { - u_char *bp = *bufp; - int len = 0; - int i, b; + caddr_t bp =*bufp; + uint32_tlen = 0; + uint32_ti, b; b = *bp++; if ( plen ) - (*plen)--; -if ( b 0x80 ) { - for ( i = 0; i (b ~0x80); i++ ) { + --(*plen); + if ( b 0x80 ) { + for ( i = 0; i (b ~0x80); ++i ) { len = len * 256 + *bp++; if ( plen ) - (*plen)--; + --(*plen); } } else len = b; @@ -492,12 +495,12 @@ */ int asn_get_encoded ( bufp, len ) - u_char **bufp; - int *len; + caddr_t *bufp; + uint32_t*len; { - u_char *bp = *bufp; - int val = 0; - int l = *len; + caddr_t bp = *bufp; + int val = 0; /* FIXME: signed? sure? */ + uint32_tl = *len; /* * Keep going while high bit is set @@ -507,7 +510,7 @@ * Each byte can represent 7 bits */ val = ( val 7 ) + ( *bp ~0x80 ); - l--; + --l; } while ( *bp++ 0x80 ); *bufp = bp; /* update buffer pointer */ @@ -526,28 +529,28 @@ * plen- pointer to PDU length or NULL if not a concern * * Returns: - * bufp- updated buffer pointer + * bufp- updated buffer pointer * plen- (possibly) updated PDU length * val - value of encoded
Re: PATCH: type errors in src-tree
Juli Mallett wrote: * De: Jens Rehsack [EMAIL PROTECTED] [ Data: 2003-03-01 ] [ Subjecte: Re: PATCH: type errors in src-tree ] Sorry for resending (3rd time), but I've found a small typo in the patch of sbin/atm/ilmid/ilmid.c - u_char **bufp; - Objid *objid; + caddr_t *bufp; + Objid *objid; I understand (and think good catch) on a number of other things, and bogus type-width assumptions being caught, etc. But this? caddr_t considered useless. Maybe you could clarify why you made the changes to use caddr_t? Of course. Very often in ilmid.c the type caddr_t was used, and nearly the same count of 'const char *'s was used. I've searched the include files for caddr_t (core address) and found it defined as 'char *', so I decided to used commonly caddr_t - maybe later I check which of them could be changed into 'c_caddr_t' for being const. But You can of couse replace all 'caddr_t' which 'char *'. Thanx, juli. Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Juli Mallett wrote: * De: Jens Rehsack [EMAIL PROTECTED] [ Data: 2003-03-01 ] [ Subjecte: Re: PATCH: type errors in src-tree ] caddr_t is discouraged, with preference of writing an actual type. caddr_t is just an obfuscation. I'm unclear what it gains in your context. Okay, sorry for misusing caddr_t. Here's a patch without any caddr_t, but with 'char *' and 'const char *'. Let me know what you're thinking about it. Jens --- sbin/atm/ilmid/ilmid.c.orig Wed Jan 1 18:48:45 2003 +++ sbin/atm/ilmid/ilmid.c Sat Mar 1 23:31:29 2003 @@ -162,7 +162,7 @@ union { int ival; /* INTEGER/TIMESTAMP */ Objid oval; /* OBJID */ - longaval; /* IPADDR */ + uint32_taval; /* IPADDR */ charsval[STRLEN]; /* OCTET */ } var; Variable*next; @@ -173,10 +173,10 @@ * which doesn't have the last three fields is the TRAP type. */ struct snmp_header { - int pdulen; - int version; + uint32_tpdulen; + uint32_tversion; charcommunity[64]; - int pdutype; + uint32_tpdutype; /* GET/GETNEXT/GETRESP/SET */ int reqid; @@ -185,11 +185,11 @@ /* TRAP */ Objid enterprise; - int ipaddr; + uint32_tipaddr; int generic_trap; int specific_trap; - int varlen; + uint32_tvarlen; Variable*head, *tail; }; @@ -279,7 +279,7 @@ * Temporary buffer for building response packets. Should help ensure * that we aren't accidently overwriting some other memory. */ -u_char Resp_Buf[1024]; +char Resp_Buf[1024]; /* * Copy the reponse into a buffer we can modify without @@ -292,7 +292,7 @@ * TRAP generic trap types */ char *Traps[] = { coldStart, warmStart, linkDown, linkUp, - authenticationFailure, egpNeighborLoss, + authenticationFailure, egpNeighborLoss, enterpriseSpecific }; @@ -320,6 +320,9 @@ */ Objid addressEntry[MAX_UNITS + 1]; +static const char ilmi_ident_str[] = ILMI; +static const size_tilmi_ident_str_len = strlen(ILMI); + /* * When this daemon started */ @@ -335,7 +338,7 @@ #defineLOG_FILE/var/log/ilmid FILE *Log; /* File descriptor for log messages */ -void set_reqid( u_char *, int ); +void set_reqid( char *, uint32_t ); void Increment_DL( int ); void Decrement_DL( int ); @@ -376,7 +379,7 @@ /* * Utility to pretty print buffer as hex dumps - * + * * Arguments: * bp - buffer pointer * len - length to pretty print @@ -387,10 +390,10 @@ */ void hexdump ( bp, len ) - u_char *bp; - int len; + const char *bp; + const uint32_t len; { - int i, j; + uint32_ti, j; /* * Print as 4 groups of four bytes. Each byte is separated @@ -443,7 +446,7 @@ * bufp- pointer to buffer pointer * plen- pointer to PDU length or NULL if not a concern * - * Returns: + * Returns: * bufp- updated buffer pointer * plen- (possibly) adjusted pdu length * len - decoded length @@ -451,21 +454,21 @@ */ int asn_get_pdu_len ( bufp, plen ) - u_char **bufp; - int *plen; + const char ** const bufp; + uint32_t* const plen; { - u_char *bp = *bufp; - int len = 0; - int i, b; + const char *bp =*bufp; + uint32_tlen = 0; + uint32_ti, b; b = *bp++; if ( plen ) - (*plen)--; -if ( b 0x80 ) { - for ( i = 0; i (b ~0x80); i++ ) { + --(*plen); + if ( b 0x80 ) { + for ( i = 0; i (b ~0x80); ++i ) { len = len * 256 + *bp++; if ( plen ) - (*plen)--; + --(*plen); } } else len = b; @@ -492,12 +495,12 @@ */ int asn_get_encoded ( bufp, len ) - u_char **bufp; - int *len; + const char ** const bufp; + uint32_t* const len; { - u_char *bp = *bufp; - int val = 0; - int l = *len; + const char *bp = *bufp; + int val = 0; /* FIXME: signed? sure? */ + uint32_tl = *len; /* * Keep going while high bit is set @@ -507,7 +510,7 @@ * Each byte can represent 7 bits */ val = ( val 7 ) + ( *bp ~0x80 ); - l
Re: PATCH: type errors in src-tree
On Sat, Mar 01, 2003 at 11:09:03PM +0100, Jens Rehsack wrote: ... - u_char **bufp; + caddr_t *bufp; ... Of course. Very often in ilmid.c the type caddr_t was used, and nearly the same count of 'const char *'s was used. I've searched the include files for caddr_t (core address) and found it defined as 'char *', so I decided to used commonly caddr_t - maybe later I check which of them could be changed into 'c_caddr_t' for being const. But You can of couse replace all 'caddr_t' which 'char *'. Shouldn't we care about u_char vs char? On some machines it matters, and on all machines compilers tend to notice and generate warnings. -- Barney Wolff http://www.databus.com/bwresume.pdf I'm available by contract or FT, in the NYC metro area or via the 'Net. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Barney Wolff wrote: On Sat, Mar 01, 2003 at 11:09:03PM +0100, Jens Rehsack wrote: Shouldn't we care about u_char vs char? On some machines it matters, and on all machines compilers tend to notice and generate warnings. Yes, usually we should. But I reviewed the code and didn't found a reason for using 'unsigned char'. I got compiler warning ('couse I have them enabled all) during compiling the file before I have finished the patch. But most of the code relies on 'char' and 'u_char' was used for storing chars only. Do you know any machine which stores signed chars different than unsigned chars? But the mixing - which was in it before - was not acceptable. Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Barney Wolff wrote: On Sat, Mar 01, 2003 at 11:09:03PM +0100, Jens Rehsack wrote: Shouldn't we care about u_char vs char? On some machines it matters, and on all machines compilers tend to notice and generate warnings. Just another question: I turned on '-ansi' and got some warning about a function declaration is not a prototype (all functions are declared in KR style). Is it preferred by the FreeBSD-Team having functions in KR styles or is the Ansi-Style on, too? Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
* De: Jens Rehsack [EMAIL PROTECTED] [ Data: 2003-03-01 ] [ Subjecte: Re: PATCH: type errors in src-tree ] Barney Wolff wrote: On Sat, Mar 01, 2003 at 11:09:03PM +0100, Jens Rehsack wrote: Shouldn't we care about u_char vs char? On some machines it matters, and on all machines compilers tend to notice and generate warnings. Just another question: I turned on '-ansi' and got some warning about a function declaration is not a prototype (all functions are declared in KR style). Is it preferred by the FreeBSD-Team having functions in KR styles or is the Ansi-Style on, too? ANSI is preferred, but mixing style with functional changes is discouraged. I get the impression your change to width-specified values has a real correlation to what things need to use, and thus it might fix brokenness with systems not ILP32. Thanx, juli. -- Juli Mallett [EMAIL PROTECTED] - AIM: BSDFlata -- IRC: juli on EFnet OpenDarwin, Mono, FreeBSD Developer - ircd-hybrid Developer, EFnet addict FreeBSD on MIPS-Anything on FreeBSD - /* XXX Nothing to see here, now. */ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Juli Mallett wrote: * De: Jens Rehsack [EMAIL PROTECTED] [ Data: 2003-03-01 ] [ Subjecte: Re: PATCH: type errors in src-tree ] Barney Wolff wrote: On Sat, Mar 01, 2003 at 11:09:03PM +0100, Jens Rehsack wrote: Shouldn't we care about u_char vs char? On some machines it matters, and on all machines compilers tend to notice and generate warnings. Just another question: I turned on '-ansi' and got some warning about a function declaration is not a prototype (all functions are declared in KR style). Is it preferred by the FreeBSD-Team having functions in KR styles or is the Ansi-Style on, too? ANSI is preferred, but mixing style with functional changes is discouraged. I get the impression your change to width-specified values has a real correlation to what things need to use, and thus it might fix brokenness with systems not ILP32. Ok, patch which complete ansi declarations appended. This fixes enumerous errors with 'declaration is not a prototype' messages. Thanx, juli. -- L i W W W i Jens Rehsack LW W W L i W W W W i nnnLiWing IT-Services L iW W W Wi n n g g i W W i n n g gFriesenstraße 2 06112 Halle g g g Tel.: +49 - 3 45 - 5 17 05 91ggg e-Mail: [EMAIL PROTECTED] Fax: +49 - 3 45 - 5 17 05 92http://www.liwing.de/ --- sbin/atm/ilmid/ilmid.c.orig Sun Mar 2 00:30:57 2003 +++ sbin/atm/ilmid/ilmid.c Sun Mar 2 00:50:40 2003 @@ -92,7 +92,8 @@ #defineASN_IPADDR 0x40 #defineASN_TIMESTAMP 0x43 -static char *Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, ASN_NULL, ASN_OBJID }; +static const char * const Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, + ASN_NULL, ASN_OBJID }; /* * Define SNMP PDU types @@ -103,8 +104,8 @@ #definePDU_TYPE_SET0xA3 #definePDU_TYPE_TRAP 0xA4 -static char *PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, GET RESPONSE, SET REQUEST, - TRAP }; +static const char * const PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, + GET RESPONSE, SET REQUEST, TRAP }; /* * Define TRAP codes @@ -162,7 +163,7 @@ union { int ival; /* INTEGER/TIMESTAMP */ Objid oval; /* OBJID */ - longaval; /* IPADDR */ + uint32_taval; /* IPADDR */ charsval[STRLEN]; /* OCTET */ } var; Variable*next; @@ -173,10 +174,10 @@ * which doesn't have the last three fields is the TRAP type. */ struct snmp_header { - int pdulen; - int version; + uint32_tpdulen; + uint32_tversion; charcommunity[64]; - int pdutype; + uint32_tpdutype; /* GET/GETNEXT/GETRESP/SET */ int reqid; @@ -185,11 +186,11 @@ /* TRAP */ Objid enterprise; - int ipaddr; + uint32_tipaddr; int generic_trap; int specific_trap; - int varlen; + uint32_tvarlen; Variable*head, *tail; }; @@ -210,7 +211,7 @@ * foresiggrp: FORE specific Objid we see alot of (being connected to FORE * switches...) */ -Objid Objids[] = { +const ObjidObjids[] = { #defineSYS_OBJID 0 {{ 8, 43, 6, 1, 2, 1, 1, 2, 0 }}, #defineUPTIME_OBJID1 @@ -279,7 +280,7 @@ * Temporary buffer for building response packets. Should help ensure * that we aren't accidently overwriting some other memory. */ -u_char Resp_Buf[1024]; +char Resp_Buf[1024]; /* * Copy the reponse into a buffer we can modify without @@ -291,8 +292,8 @@ /* * TRAP generic trap types */ -char *Traps[] = { coldStart, warmStart, linkDown, linkUp, - authenticationFailure, egpNeighborLoss, +const char *Traps[] = { coldStart, warmStart, linkDown, linkUp, + authenticationFailure, egpNeighborLoss, enterpriseSpecific }; @@ -320,6 +321,9 @@ */ Objid addressEntry[MAX_UNITS + 1]; +static const char ilmi_ident_str[] = ILMI; +static const size_tilmi_ident_str_len = strlen(ILMI); + /* * When this daemon started */ @@ -335,11 +339,11 @@ #defineLOG_FILE/var/log/ilmid FILE *Log; /* File descriptor for log messages */ -void set_reqid( u_char *, int ); -void Increment_DL( int ); -void Decrement_DL( int ); +static voidset_reqid( char *, uint32_t ); +static voidIncrement_DL( int ); +static voidDecrement_DL( int ); -static char
Re: PATCH: type errors in src-tree
Juli Mallett wrote: ANSI is preferred, but mixing style with functional changes is discouraged. I get the impression your change to width-specified values has a real correlation to what things need to use, and thus it might fix brokenness with systems not ILP32. Sorry, it's getting late here - seems that the compiler didn't cast itself to 'char const **' in line 2279. Jens -- L i W W W i Jens Rehsack LW W W L i W W W W i nnnLiWing IT-Services L iW W W Wi n n g g i W W i n n g gFriesenstraße 2 06112 Halle g g g Tel.: +49 - 3 45 - 5 17 05 91ggg e-Mail: [EMAIL PROTECTED] Fax: +49 - 3 45 - 5 17 05 92http://www.liwing.de/ --- sbin/atm/ilmid/ilmid.c.orig Sun Mar 2 00:30:57 2003 +++ sbin/atm/ilmid/ilmid.c Sun Mar 2 00:56:16 2003 @@ -92,7 +92,8 @@ #defineASN_IPADDR 0x40 #defineASN_TIMESTAMP 0x43 -static char *Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, ASN_NULL, ASN_OBJID }; +static const char * const Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, + ASN_NULL, ASN_OBJID }; /* * Define SNMP PDU types @@ -103,8 +104,8 @@ #definePDU_TYPE_SET0xA3 #definePDU_TYPE_TRAP 0xA4 -static char *PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, GET RESPONSE, SET REQUEST, - TRAP }; +static const char * const PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, + GET RESPONSE, SET REQUEST, TRAP }; /* * Define TRAP codes @@ -162,7 +163,7 @@ union { int ival; /* INTEGER/TIMESTAMP */ Objid oval; /* OBJID */ - longaval; /* IPADDR */ + uint32_taval; /* IPADDR */ charsval[STRLEN]; /* OCTET */ } var; Variable*next; @@ -173,10 +174,10 @@ * which doesn't have the last three fields is the TRAP type. */ struct snmp_header { - int pdulen; - int version; + uint32_tpdulen; + uint32_tversion; charcommunity[64]; - int pdutype; + uint32_tpdutype; /* GET/GETNEXT/GETRESP/SET */ int reqid; @@ -185,11 +186,11 @@ /* TRAP */ Objid enterprise; - int ipaddr; + uint32_tipaddr; int generic_trap; int specific_trap; - int varlen; + uint32_tvarlen; Variable*head, *tail; }; @@ -210,7 +211,7 @@ * foresiggrp: FORE specific Objid we see alot of (being connected to FORE * switches...) */ -Objid Objids[] = { +const ObjidObjids[] = { #defineSYS_OBJID 0 {{ 8, 43, 6, 1, 2, 1, 1, 2, 0 }}, #defineUPTIME_OBJID1 @@ -279,7 +280,7 @@ * Temporary buffer for building response packets. Should help ensure * that we aren't accidently overwriting some other memory. */ -u_char Resp_Buf[1024]; +char Resp_Buf[1024]; /* * Copy the reponse into a buffer we can modify without @@ -291,8 +292,8 @@ /* * TRAP generic trap types */ -char *Traps[] = { coldStart, warmStart, linkDown, linkUp, - authenticationFailure, egpNeighborLoss, +const char *Traps[] = { coldStart, warmStart, linkDown, linkUp, + authenticationFailure, egpNeighborLoss, enterpriseSpecific }; @@ -320,6 +321,9 @@ */ Objid addressEntry[MAX_UNITS + 1]; +static const char ilmi_ident_str[] = ILMI; +static const size_tilmi_ident_str_len = strlen(ILMI); + /* * When this daemon started */ @@ -335,11 +339,11 @@ #defineLOG_FILE/var/log/ilmid FILE *Log; /* File descriptor for log messages */ -void set_reqid( u_char *, int ); -void Increment_DL( int ); -void Decrement_DL( int ); +static voidset_reqid( char *, uint32_t ); +static voidIncrement_DL( int ); +static voidDecrement_DL( int ); -static char*Months[] = { Jan, Feb, Mar, Apr, May, Jun, +static const char *Months[] = { Jan, Feb, Mar, Apr, May, Jun, Jul, Aug, Sep, Oct, Nov, Dec }; /* @@ -355,14 +359,14 @@ * none * */ -void -write_timestamp() +static void +write_timestamp(void) { - time_t clock; + time_t cur_clock; struct tm *tm; - clock = time ( (time_t)NULL ); - tm = localtime ( clock ); + cur_clock = time ( (time_t)NULL ); + tm = localtime ( cur_clock ); if ( Log Debug_Level 1 ) if ( Log != stderr ) @@ -385,12
Re: PATCH: type errors in src-tree
This is an example of what I was pointing out: On Sun, Mar 02, 2003 at 01:53:33AM +0100, Jens Rehsack wrote: ... @@ -1444,22 +1420,19 @@ * none- response sent * */ -void -send_resp ( intf, Hdr, resp ) - int intf; - Snmp_Header *Hdr; - u_char *resp; +static void +send_resp ( const int intf, Snmp_Header *Hdr, char *resp ) { int n; - if ( ilmi_fd[intf] 0 ) { - n = write ( ilmi_fd[intf], (caddr_t)resp[1], resp[0] ); + if ( ilmi_fd[intf] 0 ) { /* FIXME: does ilmi_fd[intf] exists? out of range? */ + n = write ( ilmi_fd[intf], resp+1, resp[0] ); ... Here's a case where it matters whether something is u_char or char. write(2) takes a size_t as its third arg, and extension of a char to that may not be the same as for u_char, for example on Sparc. If the response is ever 127 bytes, this will fail. You're going to have to look carefully at how things are used to see when char is appropriate and when u_char is necessary. -- Barney Wolff http://www.databus.com/bwresume.pdf I'm available by contract or FT, in the NYC metro area or via the 'Net. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Barney Wolff wrote: This is an example of what I was pointing out: On Sun, Mar 02, 2003 at 01:53:33AM +0100, Jens Rehsack wrote: ... @@ -1444,22 +1420,19 @@ * none- response sent * */ -void -send_resp ( intf, Hdr, resp ) - int intf; - Snmp_Header *Hdr; - u_char *resp; +static void +send_resp ( const int intf, Snmp_Header *Hdr, char *resp ) { int n; - if ( ilmi_fd[intf] 0 ) { - n = write ( ilmi_fd[intf], (caddr_t)resp[1], resp[0] ); + if ( ilmi_fd[intf] 0 ) { /* FIXME: does ilmi_fd[intf] exists? out of range? */ + n = write ( ilmi_fd[intf], resp+1, resp[0] ); ... Here's a case where it matters whether something is u_char or char. write(2) takes a size_t as its third arg, and extension of a char to that may not be the same as for u_char, for example on Sparc. If the response is ever 127 bytes, this will fail. You're going to have to look carefully at how things are used to see when char is appropriate and when u_char is necessary. That is really right, but for those check I have to know more 'bout ATM, right? I just have detected some compiler errors using -finline-functions (yes, I'm playing with optimization options :-)). If you know a real good online-reference, one fine day I'll check it and check the entire ilmid.c code for valid signment. Jens To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: PATCH: type errors in src-tree
Barney Wolff wrote: This is an example of what I was pointing out: On Sun, Mar 02, 2003 at 01:53:33AM +0100, Jens Rehsack wrote: ... @@ -1444,22 +1420,19 @@ * none- response sent * */ -void -send_resp ( intf, Hdr, resp ) - int intf; - Snmp_Header *Hdr; - u_char *resp; +static void +send_resp ( const int intf, Snmp_Header *Hdr, char *resp ) { int n; - if ( ilmi_fd[intf] 0 ) { - n = write ( ilmi_fd[intf], (caddr_t)resp[1], resp[0] ); + if ( ilmi_fd[intf] 0 ) { /* FIXME: does ilmi_fd[intf] exists? out of range? */ + n = write ( ilmi_fd[intf], resp+1, resp[0] ); ... Here's a case where it matters whether something is u_char or char. write(2) takes a size_t as its third arg, and extension of a char to that may not be the same as for u_char, for example on Sparc. If the response is ever 127 bytes, this will fail. You're going to have to look carefully at how things are used to see when char is appropriate and when u_char is necessary. Fixed, thanks. Jens --- sbin/atm/ilmid/ilmid.c.orig Sun Mar 2 00:30:57 2003 +++ sbin/atm/ilmid/ilmid.c Sun Mar 2 01:45:06 2003 @@ -92,7 +92,8 @@ #defineASN_IPADDR 0x40 #defineASN_TIMESTAMP 0x43 -static char *Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, ASN_NULL, ASN_OBJID }; +static const char * const Var_Types[] = { , , ASN_INTEGER, , ASN_OCTET, + ASN_NULL, ASN_OBJID }; /* * Define SNMP PDU types @@ -103,8 +104,8 @@ #definePDU_TYPE_SET0xA3 #definePDU_TYPE_TRAP 0xA4 -static char *PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, GET RESPONSE, SET REQUEST, - TRAP }; +static const char * const PDU_Types[] = { GET REQUEST, GETNEXT REQUEST, + GET RESPONSE, SET REQUEST, TRAP }; /* * Define TRAP codes @@ -162,7 +163,7 @@ union { int ival; /* INTEGER/TIMESTAMP */ Objid oval; /* OBJID */ - longaval; /* IPADDR */ + uint32_taval; /* IPADDR */ charsval[STRLEN]; /* OCTET */ } var; Variable*next; @@ -173,10 +174,10 @@ * which doesn't have the last three fields is the TRAP type. */ struct snmp_header { - int pdulen; - int version; + uint32_tpdulen; + uint32_tversion; charcommunity[64]; - int pdutype; + uint32_tpdutype; /* GET/GETNEXT/GETRESP/SET */ int reqid; @@ -185,11 +186,11 @@ /* TRAP */ Objid enterprise; - int ipaddr; + uint32_tipaddr; int generic_trap; int specific_trap; - int varlen; + uint32_tvarlen; Variable*head, *tail; }; @@ -210,7 +211,7 @@ * foresiggrp: FORE specific Objid we see alot of (being connected to FORE * switches...) */ -Objid Objids[] = { +const ObjidObjids[] = { #defineSYS_OBJID 0 {{ 8, 43, 6, 1, 2, 1, 1, 2, 0 }}, #defineUPTIME_OBJID1 @@ -279,7 +280,7 @@ * Temporary buffer for building response packets. Should help ensure * that we aren't accidently overwriting some other memory. */ -u_char Resp_Buf[1024]; +char Resp_Buf[1024]; /* * Copy the reponse into a buffer we can modify without @@ -291,8 +292,8 @@ /* * TRAP generic trap types */ -char *Traps[] = { coldStart, warmStart, linkDown, linkUp, - authenticationFailure, egpNeighborLoss, +const char *Traps[] = { coldStart, warmStart, linkDown, linkUp, + authenticationFailure, egpNeighborLoss, enterpriseSpecific }; @@ -320,6 +321,9 @@ */ Objid addressEntry[MAX_UNITS + 1]; +static const char ilmi_ident_str[] = ILMI; +static const size_tilmi_ident_str_len = strlen(ILMI); + /* * When this daemon started */ @@ -335,11 +339,11 @@ #defineLOG_FILE/var/log/ilmid FILE *Log; /* File descriptor for log messages */ -void set_reqid( u_char *, int ); -void Increment_DL( int ); -void Decrement_DL( int ); +static voidset_reqid( char *, uint32_t ); +static voidIncrement_DL( int ); +static voidDecrement_DL( int ); -static char*Months[] = { Jan, Feb, Mar, Apr, May, Jun, +static const char *Months[] = { Jan, Feb, Mar, Apr, May, Jun, Jul, Aug, Sep, Oct, Nov, Dec }; /* @@ -355,14 +359,14 @@ * none * */ -void -write_timestamp() +static void +write_timestamp(void) { - time_t clock; + time_t cur_clock; struct tm *tm; - clock = time ( (time_t)NULL ); -
Re: PATCH: type errors in src-tree
On Sun, Mar 02, 2003 at 02:33:55AM +0100, Jens Rehsack wrote: ... + n = write ( ilmi_fd[intf], resp+1, resp[0] ); Here's a case where it matters whether something is u_char or char. write(2) takes a size_t as its third arg, and extension of a char to that may not be the same as for u_char, for example on Sparc. If the response is ever 127 bytes, this will fail. You're going to have to look carefully at how things are used to see when char is appropriate and when u_char is necessary. That is really right, but for those check I have to know more 'bout ATM, right? I just have detected some compiler errors using -finline-functions (yes, I'm playing with optimization options :-)). If you know a real good online-reference, one fine day I'll check it and check the entire ilmid.c code for valid signment. This has nothing to do with ATM. It has to do with how the code that's in the file uses 8-bit bytes, and how it calls standard functions. If you make changes blindly just to get rid of compiler warnings, you will introduce bugs in operation that may be difficult to find, years later. The fact that Resp_Buf needs to remain a u_char[] is just one example. -- Barney Wolff http://www.databus.com/bwresume.pdf I'm available by contract or FT, in the NYC metro area or via the 'Net. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
PATCH: type errors in src-tree
Hi, sorry for resending, but I've seen that all people who submit patches wrote 'PATCH' in their subject line. Ok, rest of the mail: I made some experiments with the optimization switches of the cc of FBSD-CURRENT, and if I turned on -finline-functions, the link of src/bin/cat fails, because of: cc -O -O3 -fforce-addr -fforce-mem -msse -mmmx -m3dnow -momit-leaf-frame-pointer -fcse-follow-jumps -fcse-skip-blocks -fgcse -frerun-cse-after-loop -fstrength-reduce -fgcse-lm -fgcse-sm -frerun-loop-opt -fschedule-insns2 -pipe -DNO_WERROR -march=athlon-tbird -Werror -Wall -Wno-format-y2k -Wno-uninitialized -Wformat=2 -Wno-format-extra-args -Werror -c /usr/src/bin/cat/cat.c cc -O -O3 -fforce-addr -fforce-mem -msse -mmmx -m3dnow -momit-leaf-frame-pointer -fcse-follow-jumps -fcse-skip-blocks -fgcse -frerun-cse-after-loop -fstrength-reduce -fgcse-lm -fgcse-sm -frerun-loop-opt -fschedule-insns2 -pipe -DNO_WERROR -march=athlon-tbird -Werror -Wall -Wno-format-y2k -Wno-uninitialized -Wformat=2 -Wno-format-extra-args -Werror -static -o cat cat.o /usr/obj/usr/src/i386/usr/lib/libc.a(atexit.o): In function `atexit': atexit.o(.text+0xc0): undefined reference to `_pthread_mutex_unlock' atexit.o(.text+0xce): undefined reference to `_pthread_mutex_lock' atexit.o(.text+0xdc): undefined reference to `_pthread_mutex_unlock' atexit.o(.text+0xfb): undefined reference to `_pthread_mutex_lock' atexit.o(.text+0x10c): undefined reference to `_pthread_mutex_unlock' atexit.o(.text+0x133): undefined reference to `_pthread_mutex_lock' /usr/obj/usr/src/i386/usr/lib/libc.a(_flock_stub.o): In function `flockfile': _flock_stub.o(.text+0x10): undefined reference to `_pthread_self' _flock_stub.o(.text+0x25): undefined reference to `_pthread_mutex_lock' /usr/obj/usr/src/i386/usr/lib/libc.a(_flock_stub.o): In function `_flockfile_debug': _flock_stub.o(.text+0x60): undefined reference to `_pthread_self' _flock_stub.o(.text+0x75): undefined reference to `_pthread_mutex_lock' /usr/obj/usr/src/i386/usr/lib/libc.a(_flock_stub.o): In function `ftrylockfile': _flock_stub.o(.text+0xb5): undefined reference to `_pthread_self' _flock_stub.o(.text+0xca): undefined reference to `_pthread_mutex_trylock' /usr/obj/usr/src/i386/usr/lib/libc.a(_flock_stub.o): In function `funlockfile': _flock_stub.o(.text+0x10d): undefined reference to `_pthread_self' _flock_stub.o(.text+0x145): undefined reference to `_pthread_mutex_unlock' *** Error code 1 Stop in /usr/src/bin/cat. *** Error code 1 Stop in /usr/src/bin. *** Error code 1 Stop in /usr/src. *** Error code 1 Stop in /usr/src. *** Error code 1 Stop in /usr/src. I patched the file src/lib/libc/gen/_pthread_stubs.c that way, that the generated functions are not static anymore. A test with the compiler using '-S' for compiling til assembly output shows no difference between static and extern declared functions. The compiler did never inline a static function unless -finline-functions is specified, and than it's done for statics and externs as well. The second error I got is from src/sbin/atm/ilmid/ilmid.c: === sbin/atm/ilmid cc -O -O3 -fforce-addr -fforce-mem -msse -mmmx -m3dnow -momit-leaf-frame-pointer -fcse-follow-jumps -fcse-skip-blocks -fgcse -frerun-cse-after-loop -fstrength-reduce -fgcse-lm -fgcse-sm -frerun-loop-opt -fschedule-insns2 -pipe -DNO_WERROR -march=athlon-tbird -I/usr/src/sbin/atm/ilmid/../../../sys -Werror -Wall -Wno-format-y2k -Wno-uninitialized -c /usr/src/sbin/atm/ilmid/ilmid.c cc1: warnings being treated as errors In file included from /usr/src/sbin/atm/ilmid/ilmid.c:861: /usr/src/sbin/atm/ilmid/ilmid.c: In function `parse_oids': /usr/src/sbin/atm/ilmid/ilmid.c:456: warning: passing arg 0 of `asn_get_pdu_len' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:867: /usr/src/sbin/atm/ilmid/ilmid.c:659: warning: passing arg 0 of `asn_get_objid' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:872: /usr/src/sbin/atm/ilmid/ilmid.c:538: warning: passing arg 0 of `asn_get_int' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:879: /usr/src/sbin/atm/ilmid/ilmid.c:659: warning: passing arg 0 of `asn_get_objid' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:882: /usr/src/sbin/atm/ilmid/ilmid.c:737: warning: passing arg 0 of `asn_get_octet' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:2144: /usr/src/sbin/atm/ilmid/ilmid.c: In function `process_get': /usr/src/sbin/atm/ilmid/ilmid.c:1833: warning: passing arg 0 of `get_local_ip' from incompatible pointer type In file included from /usr/src/sbin/atm/ilmid/ilmid.c:2328: /usr/src/sbin/atm/ilmid/ilmid.c: In function `ilmi_do_state': /usr/src/sbin/atm/ilmid/ilmid.c:1074: warning: passing arg 0 of `oid_ncmp' from incompatible pointer type /usr/src/sbin/atm/ilmid/ilmid.c:1074: warning: passing arg 0 of `oid_ncmp' from incompatible pointer type In file included from