aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGES4
-rw-r--r--RELEASE-NOTES1
-rw-r--r--docs/KNOWN_BUGS2
-rw-r--r--lib/tftp.c111
4 files changed, 57 insertions, 61 deletions
diff --git a/CHANGES b/CHANGES
index c4f050072..03fd1ba92 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,10 @@
Changelog
+Daniel (8 May 2006)
+- Fixed known bug #28. The TFTP code no longer assumes a packed struct and
+ thus works reliably on more platforms.
+
Daniel (5 May 2006)
- Roland Blom filed bug report #1481217
(http://curl.haxx.se/bug/view.cgi?id=1481217), with follow-ups by Michele
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index f4f2230d7..208e5b1be 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -20,6 +20,7 @@ This release includes the following changes:
This release includes the following bugfixes:
+ o TFTP works in a more portable fashion (== on more platforms)
o WSAGetLastError() is now used (better) on Windows
o GnuTLS non-block case that could cause data trashing
o deflate code survives lack of zlib header
diff --git a/docs/KNOWN_BUGS b/docs/KNOWN_BUGS
index 9149b996a..ba0bfa4e5 100644
--- a/docs/KNOWN_BUGS
+++ b/docs/KNOWN_BUGS
@@ -33,8 +33,6 @@ may have been fixed since this was written!
See http://curl.haxx.se/bug/view.cgi?id=1371118
-28. The TFTP code is not portable and will fail on some architectures.
-
26. NTLM authentication using SSPI (on Windows) when (lib)curl is running in
"system context" will make it use wrong(?) user name - at least when compared
to what winhttp does. See http://curl.haxx.se/bug/view.cgi?id=1281867
diff --git a/lib/tftp.c b/lib/tftp.c
index d8a23b036..780cceea5 100644
--- a/lib/tftp.c
+++ b/lib/tftp.c
@@ -122,23 +122,7 @@ typedef enum {
} tftp_error_t;
typedef struct tftp_packet {
- unsigned short event;
- union {
- struct {
- unsigned char data[512];
- } request;
- struct {
- unsigned short block;
- unsigned char data[512];
- } data;
- struct {
- unsigned short block;
- } ack;
- struct {
- unsigned short code;
- unsigned char data[512];
- } error;
- } u;
+ unsigned char data[516];
} tftp_packet_t;
typedef struct tftp_state_data {
@@ -199,7 +183,8 @@ void tftp_set_timeouts(tftp_state_data_t *state)
/* Compute the re-start interval to suit the timeout */
state->retry_time = timeout/state->retry_max;
- if(state->retry_time<1) state->retry_time=1;
+ if(state->retry_time<1)
+ state->retry_time=1;
}
else {
@@ -215,12 +200,16 @@ void tftp_set_timeouts(tftp_state_data_t *state)
state->retry_max = timeout/15;
}
/* But bound the total number */
- if(state->retry_max<3) state->retry_max=3;
- if(state->retry_max>50) state->retry_max=50;
+ if(state->retry_max<3)
+ state->retry_max=3;
+
+ if(state->retry_max>50)
+ state->retry_max=50;
/* Compute the re-ACK interval to suit the timeout */
state->retry_time = timeout/state->retry_max;
- if(state->retry_time<1) state->retry_time=1;
+ if(state->retry_time<1)
+ state->retry_time=1;
infof(data, "set timeouts for state %d; Total %d, retry %d maxtry %d\n",
state->state, (state->max_time-state->start_time),
@@ -235,6 +224,29 @@ void tftp_set_timeouts(tftp_state_data_t *state)
*
**********************************************************/
+static void setpacketevent(tftp_packet_t *packet, unsigned short num)
+{
+ packet->data[0] = (num >> 8);
+ packet->data[1] = (num & 0xff);
+}
+
+
+static void setpacketblock(tftp_packet_t *packet, unsigned short num)
+{
+ packet->data[2] = (num >> 8);
+ packet->data[3] = (num & 0xff);
+}
+
+static unsigned short getrpacketevent(tftp_packet_t *packet)
+{
+ return (packet->data[0] << 8) | packet->data[1];
+}
+
+static unsigned short getrpacketblock(tftp_packet_t *packet)
+{
+ return (packet->data[2] << 8) | packet->data[3];
+}
+
static void tftp_send_first(tftp_state_data_t *state, tftp_event_t event)
{
int sbytes;
@@ -260,18 +272,18 @@ static void tftp_send_first(tftp_state_data_t *state, tftp_event_t event)
if(data->set.upload) {
/* If we are uploading, send an WRQ */
- state->spacket.event = htons(TFTP_EVENT_WRQ);
+ setpacketevent(&state->spacket, TFTP_EVENT_WRQ);
filename = curl_easy_unescape(data, filename, 0, NULL);
- state->conn->upload_fromhere = (char *)state->spacket.u.data.data;
+ state->conn->upload_fromhere = (char *)&state->spacket.data[2];
if(data->set.infilesize != -1)
Curl_pgrsSetUploadSize(data, data->set.infilesize);
}
else {
/* If we are downloading, send an RRQ */
- state->spacket.event = htons(TFTP_EVENT_RRQ);
+ setpacketevent(&state->spacket, TFTP_EVENT_RRQ);
}
- snprintf((char *)state->spacket.u.request.data,
- sizeof(state->spacket.u.request.data),
+ snprintf((char *)&state->spacket.data[2],
+ 512,
"%s%c%s%c", filename, '\0', mode, '\0');
sbytes = 4 + (int)strlen(filename) + (int)strlen(mode);
sbytes = sendto(state->sockfd, (void *)&state->spacket,
@@ -325,7 +337,7 @@ static void tftp_rx(tftp_state_data_t *state, tftp_event_t event)
case TFTP_EVENT_DATA:
/* Is this the block we expect? */
- rblock = ntohs(state->rpacket.u.data.block);
+ rblock = getrpacketblock(&state->rpacket);
if ((state->block+1) != rblock) {
/* No, log it, up the retry count and fail if over the limit */
infof(data,
@@ -340,9 +352,9 @@ static void tftp_rx(tftp_state_data_t *state, tftp_event_t event)
/* This is the expected block. Reset counters and ACK it. */
state->block = rblock;
state->retries = 0;
- state->spacket.event = htons(TFTP_EVENT_ACK);
- state->spacket.u.ack.block = htons(state->block);
- sbytes = sendto(state->sockfd, (void *)&state->spacket,
+ setpacketevent(&state->spacket, TFTP_EVENT_ACK);
+ setpacketblock(&state->spacket, state->block);
+ sbytes = sendto(state->sockfd, (void *)state->spacket.data,
4, SEND_4TH_ARG,
(struct sockaddr *)&state->remote_addr,
state->remote_addrlen);
@@ -409,7 +421,7 @@ static void tftp_tx(tftp_state_data_t *state, tftp_event_t event)
case TFTP_EVENT_ACK:
/* Ack the packet */
- rblock = ntohs(state->rpacket.u.data.block);
+ rblock = getrpacketblock(&state->rpacket);
if(rblock != state->block) {
/* This isn't the expected block. Log it and up the retry counter */
@@ -428,14 +440,14 @@ static void tftp_tx(tftp_state_data_t *state, tftp_event_t event)
block */
state->block++;
state->retries = 0;
- state->spacket.event = htons(TFTP_EVENT_DATA);
- state->spacket.u.ack.block = htons(state->block);
+ setpacketevent(&state->spacket, TFTP_EVENT_DATA);
+ setpacketblock(&state->spacket, state->block);
if(state->block > 1 && state->sbytes < 512) {
state->state = TFTP_STATE_FIN;
return;
}
Curl_fillreadbuffer(state->conn, 512, &state->sbytes);
- sbytes = sendto(state->sockfd, (void *)&state->spacket,
+ sbytes = sendto(state->sockfd, (void *)state->spacket.data,
4+state->sbytes, SEND_4TH_ARG,
(struct sockaddr *)&state->remote_addr,
state->remote_addrlen);
@@ -529,28 +541,9 @@ CURLcode Curl_tftp_connect(struct connectdata *conn, bool *done)
tftp_state_data_t *state;
int rc;
- /*
- * The TFTP code is not portable because it sends C structs directly over
- * the wire. Since C gives compiler writers a wide latitude in padding and
- * aligning structs, this fails on many architectures (e.g. ARM).
- *
- * The only portable way to fix this is to copy each struct item into a
- * flat buffer and send the flat buffer instead of the struct. The
- * alternative, trying to get the compiler to eliminate padding bytes
- * within the struct, is a nightmare to maintain (each compiler does it
- * differently), and is still not guaranteed to work because some
- * architectures can't handle the resulting alignment.
- *
- * This check can be removed once the code has been fixed.
- */
- if(sizeof(struct tftp_packet) != 516) {
- failf(conn->data, "tftp not supported on this architecture");
- return CURLE_FAILED_INIT;
- }
-
- if((state = conn->proto.tftp = calloc(sizeof(tftp_state_data_t), 1))==NULL) {
+ state = conn->proto.tftp = calloc(sizeof(tftp_state_data_t), 1);
+ if(!state)
return CURLE_OUT_OF_MEMORY;
- }
state->conn = conn;
state->sockfd = state->conn->sock[FIRSTSOCKET];
@@ -678,17 +671,17 @@ CURLcode Curl_tftp(struct connectdata *conn, bool *done)
} else {
/* The event is given by the TFTP packet time */
- event = (tftp_event_t)ntohs(state->rpacket.event);
+ event = (tftp_event_t)getrpacketevent(&state->rpacket);
switch(event) {
case TFTP_EVENT_DATA:
if (state->rbytes > 4)
Curl_client_write(data, CLIENTWRITE_BODY,
- (char *)state->rpacket.u.data.data, state->rbytes-4);
+ (char *)&state->rpacket.data[4], state->rbytes-4);
break;
case TFTP_EVENT_ERROR:
- state->error = (tftp_error_t)ntohs(state->rpacket.u.error.code);
- infof(conn->data, "%s\n", (char *)state->rpacket.u.error.data);
+ state->error = (tftp_error_t)getrpacketblock(&state->rpacket);
+ infof(conn->data, "%s\n", (char *)&state->rpacket.data[4]);
break;
case TFTP_EVENT_ACK:
break;