> I've attached the patch that Willem sent me this morning. > I'm cherry-picking them in my comments below. Ah Life! > Here are a few comments that I have to start the clock:
Well lets see if I can follow suite..... In general int => size_t changes are there because of compiler warnings for funtions which expect size_t parameters. And again on 64bits int <> size_t > 1. Forget the tcpTable/udpTable patches. They do not > apply cleanly to net-snmp-5.1.1, and they duplicate > changes that have already been applied to 5.1.2.pre2, > which does work on FreeBSD 4.x/5.x. Fine with me. I had a hard time figuring out the TCP version until I saw that the data was there, but we were using the wrong structure. And then the UDP version followed the same pattern. > 2. The agent/agent_trap.c patch is interesting. V5-1-patches > already declares sinkport as an integer, which is proper > for the "atoi()" call. However, it should cast that > value to (u_short) when calling the createXXsession calls. > The main branch should also use int for sinkport, and > cast the result to u_short. Well atoi() returns an integer, so it is not clear what happens once you force it into an u_short. Second u_short can not be negative, so the test (sinkport < 1) is always false, as is the second test since max(u_short) == 0xffff. So the complete test is bogus. This was pointed out by the compiler, which made me look into what is going on. Since typeconversion are THE issue in porting to 64bits, it was very easy to spot. There are more places, but I suggest that copy&paste code like this is put in either small local (inline) routines or macros. Another place with multiple Copy&Paste is in tcpTable/udpTable > 3. The agent/mibgroup/mibII/ip.c, function int ip_handler, line 892 > return NULL is bad, as you've noticed. > the value (int) NULL is assumed to be 0, [not universally true!] This is true. I just forced the conversion to please the compiler. Which i did since the function tries more often to return pointers. So I saw this as a "signal", but 0 certainly works. > How about replacing the return with "continue", > as other errored results do in this function ? I personally am not a favourite of continue, or even a hard return from a function out of a case. Being raised in Pascal I'd like to terminate a switch properly. Please solve it as you see fit, it is almost semantics since good compiler optimize most out. Significant here is however the change, in ip_load: +/* XXX int magic = (int) vmagic; Does not work on 64Bit*/ + long magic = (long)(vmagic); And this I probably would replace by caddr_t, or some equivalent. Since this is most likely portable. > 4. The patch for hr_print.c is not good. I believe the change that is > is 5.1.2.pre2 makes the function more robust, but that belief hasn't > been tested on a system with many printers %^) This is where snmpd coredumps on me with an illegal reference. Something is wrong..... I assume that cgetnext allocates the buffer, which is here freed. But looking at the address in GDB I saw that it was not an address that was somewhere in the heap, but was allocated on the stack. Trying to free that makes things barf. Still haven't had time to load 5.1.2pre2.... > 5. Question: when you apply the patch to mibII/ipv6.c, does this fix > the "OID not increasing" bug 735912 ? that one is to circumvent a possible bug in FreeBSD where the first string into the sysctlbyname call is not treated as constant. And it disappears once the call has been issued 1 time. So the assignment to p, is just to save the address for the second call. Clearly a piece of code that could do with a macro, or a small local routine. > 6. There is an attached patch that removes the CPP macro from > mibII/var_route.c. This source includes util_funcs.h, which > defines the CPP macro "satosin" differently. > Does this patch fix problems with the addresses seen in FreeBSD ? > > mibII/var_route.c and mibII/ipAddr.c both define a CPP macro > named "satosin", like this : > #define satosin(sa) ((struct sockaddr_in *)(sa)) > which differs from the CPP macro defined in util_funcs.h : > #define satosin(x) ((struct sockaddr_in *) &(x)) > > mibII/var_route.c includes util_funcs.h, and there is an > attached patch that removes the CPP macro from mibII/var_route.c. > > In both sources, sa is declared as struct sockaddr *sa, > and satosin is used to copy a sin_addr member to a struct in_addr. In my version of FreeBSD satosin is already defined.... # find /usr/include -type f | xargs grep -n satosin /usr/include/netinet/in.h:567:#define satosin(sa) ((struct sockaddr_in *)(sa)) /usr/include/netinet6/in6.h:628:#define satosin6(sa) ((struct sockaddr_in6 *)(sa)) /usr/include/netinet6/in6_pcb.h:69:#define satosin6(sa) ((struct sockaddr_in6 *)(sa)) Things seem te be equal... So either make the inclusion conditional or undo a previous definition. > 7. Regarding the snmplib/scapi.c patch, does this match the version > of OpenSSL you are using ? # find /usr/include -type f | xargs grep -n EVP_DigestFinal_ex /usr/include/openssl/evp.h:524:int EVP_DigestFinal_ex(EVP_MD_CTX *ctx,unsigned char *md,unsigned int *s); Which looks like the parameters need to be references..... As they are passed in the function header. However the compiler was complaining and this shut it up. But now I think it was more complaining about the difference between (size_t*) <> (unsigned int*) Which would require a change in the parameters of sc_hash, or do an implicit conversion.... --WjW ------------------------------------------------------- This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com _______________________________________________ Net-snmp-coders mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
