diff --git a/CHANGES b/CHANGES index a6b0dec..4c2df40 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,10 @@ $Id$ +Changes in 0.34: + Additional Solaris gcc compiler bug workarounds; these + actually make the code somewhat cleaner. + + Changes in 0.33: Even better error messages. diff --git a/tftp/tftp.c b/tftp/tftp.c index b6c1f5f..0e615e5 100644 --- a/tftp/tftp.c +++ b/tftp/tftp.c @@ -89,6 +89,7 @@ tftp_sendfile(int fd, const char *name, const char *mode) struct sockaddr_in from; int fromlen; FILE *file; + u_short ap_opcode, ap_block; startclock(); /* start stat's clock */ dp = r_init(); /* reset fillbuf/read-ahead code */ @@ -141,17 +142,17 @@ tftp_sendfile(int fd, const char *name, const char *mode) if (trace) tpacket("received", ap, n); /* should verify packet came from server */ - ap->th_opcode = ntohs(ap->th_opcode); - ap->th_block = ntohs(ap->th_block); - if (ap->th_opcode == ERROR) { - printf("Error code %d: %s\n", ap->th_code, + ap_opcode = ntohs((u_short)ap->th_opcode); + ap_block = ntohs((u_short)ap->th_block); + if (ap_opcode == ERROR) { + printf("Error code %d: %s\n", ap_block, ap->th_msg); goto abort; } - if (ap->th_opcode == ACK) { + if (ap_opcode == ACK) { int j; - if (ap->th_block == block) { + if (ap_block == block) { break; } /* On an error, try to synchronize @@ -197,6 +198,7 @@ tftp_recvfile(int fd, const char *name, const char *mode) int fromlen; FILE *file; volatile int convert; /* true if converting crlf -> lf */ + u_short dp_opcode, dp_block; startclock(); dp = w_init(); @@ -214,7 +216,7 @@ tftp_recvfile(int fd, const char *name, const char *mode) firsttrip = 0; } else { ap->th_opcode = htons((u_short)ACK); - ap->th_block = htons((u_short)(block)); + ap->th_block = htons((u_short)block); size = 4; block++; } @@ -246,17 +248,16 @@ send_ack: if (trace) tpacket("received", dp, n); /* should verify client address */ - dp->th_opcode = ntohs(dp->th_opcode); - dp->th_block = ntohs(dp->th_block); - if (dp->th_opcode == ERROR) { - printf("Error code %d: %s\n", dp->th_code, - dp->th_msg); - goto abort; + dp_opcode = ntohs((u_short)dp->th_opcode); + dp_block = ntohs((u_short)dp->th_block); + if (dp_opcode == ERROR) { + printf("Error code %d: %s\n", dp_block, dp->th_msg); + goto abort; } - if (dp->th_opcode == DATA) { + if (dp_opcode == DATA) { int j; - if (dp->th_block == block) { + if (dp_block == block) { break; /* have next packet */ } /* On an error, try to synchronize @@ -266,7 +267,7 @@ send_ack: if (j && trace) { printf("discarded %d packets\n", j); } - if (dp->th_block == (block-1)) { + if (dp_block == (block-1)) { goto send_ack; /* resend ack */ } } @@ -308,21 +309,19 @@ makerequest(int request, const char *name, return (cp - (char *)tp); } -struct errmsg { - int e_code; - const char *e_msg; -} errmsgs[] = { - { EUNDEF, "Undefined error code" }, - { ENOTFOUND, "File not found" }, - { EACCESS, "Access violation" }, - { ENOSPACE, "Disk full or allocation exceeded" }, - { EBADOP, "Illegal TFTP operation" }, - { EBADID, "Unknown transfer ID" }, - { EEXISTS, "File already exists" }, - { ENOUSER, "No such user" }, - { EOPTNEG, "Failure to negotiate RFC2347 options" }, - { -1, 0 } +static const char * const errmsgs[] = +{ + "Undefined error code", /* 0 - EUNDEF */ + "File not found", /* 1 - ENOTFOUND */ + "Access denied", /* 2 - EACCESS */ + "Disk full or allocation exceeded", /* 3 - ENOSPACE */ + "Illegal TFTP operation", /* 4 - EBADOP */ + "Unknown transfer ID", /* 5 - EBADID */ + "File already exists", /* 6 - EEXISTS */ + "No such user", /* 7 - ENOUSER */ + "Failure to negotiate RFC2347 options" /* 8 - EOPTNEG */ }; +#define ERR_CNT (sizeof(errmsgs)/sizeof(const char *)) /* * Send a nak packet (error message). @@ -333,26 +332,28 @@ struct errmsg { static void nak(int error, const char *msg) { - struct errmsg *pe; struct tftphdr *tp; int length; tp = (struct tftphdr *)ackbuf; tp->th_opcode = htons((u_short)ERROR); tp->th_code = htons((u_short)error); - if ( !msg ) { - if ( error >= 100 ) { + + if ( error >= 100 ) { + /* This is a Unix errno+100 */ + if ( !msg ) msg = strerror(error - 100); - tp->th_code = EUNDEF; - } else { - for (pe = errmsgs; pe->e_code >= 0; pe++) { - if (pe->e_code == error) { - msg = pe->e_msg; - break; - } - } - } + error = EUNDEF; + } else { + if ( (unsigned)error >= ERR_CNT ) + error = EUNDEF; + + if ( !msg ) + msg = errmsgs[error]; } + + tp->th_code = htons((u_short)error); + length = strlen(msg)+1; memcpy(tp->th_msg, msg, length); length += 4; /* Add space for header */ @@ -370,7 +371,7 @@ tpacket(const char *s, struct tftphdr *tp, int n) static const char *opcodes[] = { "#0", "RRQ", "WRQ", "DATA", "ACK", "ERROR", "OACK" }; char *cp, *file; - u_short op = ntohs(tp->th_opcode); + u_short op = ntohs((u_short)tp->th_opcode); if (op < RRQ || op > ERROR) printf("%s opcode=%x ", s, op); diff --git a/tftpd/tftpd.c b/tftpd/tftpd.c index 180a02f..fc9a843 100644 --- a/tftpd/tftpd.c +++ b/tftpd/tftpd.c @@ -270,6 +270,7 @@ main(int argc, char **argv) #ifdef WITH_REGEX char *rewrite_file = NULL; #endif + u_short tp_opcode; /* basename() is way too much of a pain from a portability standpoint */ @@ -677,8 +678,8 @@ main(int argc, char **argv) exit(EX_IOERR); } tp = (struct tftphdr *)buf; - tp->th_opcode = ntohs(tp->th_opcode); - if (tp->th_opcode == RRQ || tp->th_opcode == WRQ) + tp_opcode = ntohs(tp->th_opcode); + if (tp_opcode == RRQ || tp_opcode == WRQ) tftp(tp, n); exit(0); } @@ -713,11 +714,12 @@ tftp(struct tftphdr *tp, int size) char *origfilename; char *filename, *mode = NULL; const char *errmsgptr; + u_short tp_opcode = ntohs(tp->th_opcode); char *val = NULL, *opt = NULL; char *ap = ackbuf + 2; - ((struct tftphdr *)ackbuf)->th_opcode = ntohs(OACK); + ((struct tftphdr *)ackbuf)->th_opcode = htons(OACK); origfilename = cp = (char *) &(tp->th_stuff); argn = 0; @@ -749,21 +751,21 @@ tftp(struct tftphdr *tp, int size) exit(0); } if ( !(filename = - (*pf->f_rewrite)(origfilename, tp->th_opcode, &errmsgptr)) ) { + (*pf->f_rewrite)(origfilename, tp_opcode, &errmsgptr)) ) { nak(EACCESS, errmsgptr); /* File denied by mapping rule */ exit(0); } if ( verbosity >= 1 ) { if ( filename == origfilename || !strcmp(filename, origfilename) ) syslog(LOG_NOTICE, "%s from %s filename %s\n", - tp->th_opcode == WRQ ? "WRQ" : "RRQ", + tp_opcode == WRQ ? "WRQ" : "RRQ", inet_ntoa(from.sin_addr), filename); else syslog(LOG_NOTICE, "%s from %s filename %s remapped to %s\n", - tp->th_opcode == WRQ ? "WRQ" : "RRQ", + tp_opcode == WRQ ? "WRQ" : "RRQ", inet_ntoa(from.sin_addr), origfilename, filename); } - ecode = (*pf->f_validate)(filename, tp->th_opcode, pf, &errmsgptr); + ecode = (*pf->f_validate)(filename, tp_opcode, pf, &errmsgptr); if (ecode) { nak(ecode, errmsgptr); exit(0); @@ -783,12 +785,12 @@ tftp(struct tftphdr *tp, int size) } if ( ap != (ackbuf+2) ) { - if ( tp->th_opcode == WRQ ) + if ( tp_opcode == WRQ ) (*pf->f_recv)(pf, (struct tftphdr *)ackbuf, ap-ackbuf); else (*pf->f_send)(pf, (struct tftphdr *)ackbuf, ap-ackbuf); } else { - if (tp->th_opcode == WRQ) + if (tp_opcode == WRQ) (*pf->f_recv)(pf, NULL, 0); else (*pf->f_send)(pf, NULL, 0); @@ -1153,6 +1155,7 @@ tftp_sendfile(struct formats *pf, struct tftphdr *oap, int oacklen) struct tftphdr *dp; struct tftphdr *ap; /* ack packet */ static u_short block = 1; /* Static to avoid longjmp funnies */ + u_short ap_opcode, ap_block; int size, n; if (oap) { @@ -1170,15 +1173,15 @@ tftp_sendfile(struct formats *pf, struct tftphdr *oap, int oacklen) goto abort; } ap = (struct tftphdr *)ackbuf; - ap->th_opcode = ntohs((u_short)ap->th_opcode); - ap->th_block = ntohs((u_short)ap->th_block); + ap_opcode = ntohs((u_short)ap->th_opcode); + ap_block = ntohs((u_short)ap->th_block); - if (ap->th_opcode == ERROR) { + if (ap_opcode == ERROR) { syslog(LOG_WARNING, "tftp: client does not accept options\n"); goto abort; } - if (ap->th_opcode == ACK) { - if (ap->th_block == 0) + if (ap_opcode == ACK) { + if (ap_block == 0) break; /* Resynchronize with the other side */ (void)synchnet(peer); @@ -1211,14 +1214,14 @@ tftp_sendfile(struct formats *pf, struct tftphdr *oap, int oacklen) goto abort; } ap = (struct tftphdr *)ackbuf; - ap->th_opcode = ntohs((u_short)ap->th_opcode); - ap->th_block = ntohs((u_short)ap->th_block); + ap_opcode = ntohs((u_short)ap->th_opcode); + ap_block = ntohs((u_short)ap->th_block); - if (ap->th_opcode == ERROR) + if (ap_opcode == ERROR) goto abort; - if (ap->th_opcode == ACK) { - if (ap->th_block == block) { + if (ap_opcode == ACK) { + if (ap_block == block) { break; } /* Re-synchronize with the other side */ @@ -1258,6 +1261,7 @@ tftp_recvfile(struct formats *pf, struct tftphdr *oap, int oacklen) static struct tftphdr *ap; /* ack buffer */ static u_short block = 0; static int acksize; + u_short dp_opcode, dp_block; dp = w_init(); do { @@ -1286,17 +1290,17 @@ tftp_recvfile(struct formats *pf, struct tftphdr *oap, int oacklen) syslog(LOG_WARNING, "tftpd: read: %m"); goto abort; } - dp->th_opcode = ntohs((u_short)dp->th_opcode); - dp->th_block = ntohs((u_short)dp->th_block); - if (dp->th_opcode == ERROR) + dp_opcode = ntohs((u_short)dp->th_opcode); + dp_block = ntohs((u_short)dp->th_block); + if (dp_opcode == ERROR) goto abort; - if (dp->th_opcode == DATA) { - if (dp->th_block == block) { + if (dp_opcode == DATA) { + if (dp_block == block) { break; /* normal */ } /* Re-synchronize with the other side */ (void) synchnet(peer); - if (dp->th_block == (block-1)) + if (dp_block == (block-1)) goto send_ack; /* rexmit */ } } @@ -1320,8 +1324,8 @@ tftp_recvfile(struct formats *pf, struct tftphdr *oap, int oacklen) timeout_quit = 0; if (n >= 4 && /* if read some data */ - dp->th_opcode == DATA && /* and got a data block */ - block == dp->th_block) { /* then my last ack was lost */ + dp_opcode == DATA && /* and got a data block */ + block == dp_block) { /* then my last ack was lost */ (void) send(peer, ackbuf, 4, 0); /* resend final ack */ } abort: @@ -1356,18 +1360,22 @@ nak(int error, const char *msg) tp = (struct tftphdr *)buf; tp->th_opcode = htons((u_short)ERROR); - tp->th_code = htons((u_short)error); - if ( !msg ) { - if ( error >= 100 ) { - /* This is a Unix errno+100 */ + + if ( error >= 100 ) { + /* This is a Unix errno+100 */ + if ( !msg ) msg = strerror(error - 100); - tp->th_code = EUNDEF; - } else { - if ( (unsigned)error >= ERR_CNT ) - tp->th_code = error = EUNDEF; + error = EUNDEF; + } else { + if ( (unsigned)error >= ERR_CNT ) + error = EUNDEF; + + if ( !msg ) msg = errmsgs[error]; - } } + + tp->th_code = htons((u_short)error); + length = strlen(msg)+1; memcpy(tp->th_msg, msg, length); length += 4; /* Add space for header */