| Message ID | 1526658340-1992-5-git-send-email-thuth@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support network booting with pxelinux.cfg files | expand |
On 19/5/18 1:45 am, Thomas Huth wrote: > This way we can easily re-use the rc --> string translation in later > patches. > > Reviewed-by: Greg Kurz <groug@kaod.org> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > lib/libnet/netload.c | 79 +++------------------------------------ > lib/libnet/tftp.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/libnet/tftp.h | 2 + > 3 files changed, 111 insertions(+), 73 deletions(-) > > diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c > index 407d173..ba1008c 100644 > --- a/lib/libnet/netload.c > +++ b/lib/libnet/netload.c > @@ -414,79 +414,12 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len) > if (rc > 0) { > printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, > rc / 1024); > - } else if (rc == -1) { > - netload_error(0x3003, "unknown TFTP error"); > - return -103; > - } else if (rc == -2) { > - netload_error(0x3004, "TFTP buffer of %d bytes " > - "is too small for %s", > - len, fnip->filename); > - return -104; > - } else if (rc == -3) { > - netload_error(0x3009, "file not found: %s", > - fnip->filename); > - return -108; > - } else if (rc == -4) { > - netload_error(0x3010, "TFTP access violation"); > - return -109; > - } else if (rc == -5) { > - netload_error(0x3011, "illegal TFTP operation"); > - return -110; > - } else if (rc == -6) { > - netload_error(0x3012, "unknown TFTP transfer ID"); > - return -111; > - } else if (rc == -7) { > - netload_error(0x3013, "no such TFTP user"); > - return -112; > - } else if (rc == -8) { > - netload_error(0x3017, "TFTP blocksize negotiation failed"); > - return -116; > - } else if (rc == -9) { > - netload_error(0x3018, "file exceeds maximum TFTP transfer size"); > - return -117; > - } else if (rc <= -10 && rc >= -15) { > - const char *icmp_err_str; > - switch (rc) { > - case -ICMP_NET_UNREACHABLE - 10: > - icmp_err_str = "net unreachable"; > - break; > - case -ICMP_HOST_UNREACHABLE - 10: > - icmp_err_str = "host unreachable"; > - break; > - case -ICMP_PROTOCOL_UNREACHABLE - 10: > - icmp_err_str = "protocol unreachable"; > - break; > - case -ICMP_PORT_UNREACHABLE - 10: > - icmp_err_str = "port unreachable"; > - break; > - case -ICMP_FRAGMENTATION_NEEDED - 10: > - icmp_err_str = "fragmentation needed and DF set"; > - break; > - case -ICMP_SOURCE_ROUTE_FAILED - 10: > - icmp_err_str = "source route failed"; > - break; > - default: > - icmp_err_str = " UNKNOWN"; > - break; > - } > - netload_error(0x3005, "ICMP ERROR \"%s\"", icmp_err_str); > - return -105; > - } else if (rc == -40) { > - netload_error(0x3014, "TFTP error occurred after " > - "%d bad packets received", > - tftp_err.bad_tftp_packets); > - return -113; > - } else if (rc == -41) { > - netload_error(0x3015, "TFTP error occurred after " > - "missing %d responses", > - tftp_err.no_packets); > - return -114; > - } else if (rc == -42) { > - netload_error(0x3016, "TFTP error missing block %d, " > - "expected block was %d", > - tftp_err.blocks_missed, > - tftp_err.blocks_received); > - return -115; > + } else { > + int ecode; > + const char *errstr = NULL; > + rc = tftp_get_error_info(fnip, &tftp_err, rc, &errstr, &ecode); > + if (errstr) > + netload_error(ecode, errstr); > } > > return rc; > diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c > index b7121fe..ec8d631 100644 > --- a/lib/libnet/tftp.c > +++ b/lib/libnet/tftp.c > @@ -690,3 +690,106 @@ int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, > return 0; > } > } > + > +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, tftperr is not used. > + const char **errstr, int *ecode) > +{ > + static char estrbuf[80]; > + int dummy; > + > + if (!ecode) > + ecode = &dummy; Nit: the other caller could also allocate this on the stack. > + > + if (rc == -1) { > + *ecode = 0x3003; > + *errstr = "unknown TFTP error"; > + return -103; > + } else if (rc == -2) { > + *ecode = 0x3004; > + sprintf(estrbuf, "TFTP buffer of %d bytes is too small for %s", > + len, fnip->filename); > + *errstr = estrbuf; > + return -104; > + } else if (rc == -3) { > + *ecode = 0x3009; > + sprintf(estrbuf, "file not found: %s", fnip->filename); estrbuf is 80 bytes, filename_ip::filename is 256. vsnprintf or increase the estrbuf size or simply malloc it. > + *errstr = estrbuf; > + return -108; > + } else if (rc == -4) { > + *ecode = 0x3010; > + *errstr = "TFTP access violation"; > + return -109; > + } else if (rc == -5) { > + *ecode = 0x3011; > + *errstr = "illegal TFTP operation"; > + return -110; > + } else if (rc == -6) { > + *ecode = 0x3012; > + *errstr = "unknown TFTP transfer ID"; > + return -111; > + } else if (rc == -7) { > + *ecode = 0x3013; > + *errstr = "no such TFTP user"; > + return -112; > + } else if (rc == -8) { > + *ecode = 0x3017; > + *errstr = "TFTP blocksize negotiation failed"; > + return -116; > + } else if (rc == -9) { > + *ecode = 0x3018; > + *errstr = "file exceeds maximum TFTP transfer size"; > + return -117; > + } else if (rc <= -10 && rc >= -15) { > + const char *icmp_err_str; > + switch (rc) { > + case -ICMP_NET_UNREACHABLE - 10: > + icmp_err_str = "net unreachable"; > + break; > + case -ICMP_HOST_UNREACHABLE - 10: > + icmp_err_str = "host unreachable"; > + break; > + case -ICMP_PROTOCOL_UNREACHABLE - 10: > + icmp_err_str = "protocol unreachable"; > + break; > + case -ICMP_PORT_UNREACHABLE - 10: > + icmp_err_str = "port unreachable"; > + break; > + case -ICMP_FRAGMENTATION_NEEDED - 10: > + icmp_err_str = "fragmentation needed and DF set"; > + break; > + case -ICMP_SOURCE_ROUTE_FAILED - 10: > + icmp_err_str = "source route failed"; > + break; > + default: > + icmp_err_str = "UNKNOWN"; > + break; > + } > + *ecode = 0x3005; > + sprintf(estrbuf, "ICMP ERROR \"%s\"", icmp_err_str); > + *errstr = estrbuf; > + return -105; > + } else if (rc == -40) { > + *ecode = 0x3014; > + sprintf(estrbuf, > + "TFTP error occurred after %d bad packets received", > + tftp_err->bad_tftp_packets); > + *errstr = estrbuf; > + return -113; > + } else if (rc == -41) { > + *ecode = 0x3015; > + sprintf(estrbuf, > + "TFTP error occurred after missing %d responses", > + tftp_err->no_packets); > + *errstr = estrbuf; > + return -114; > + } else if (rc == -42) { > + *ecode = 0x3016; > + sprintf(estrbuf, > + "TFTP error missing block %d, expected block was %d", > + tftp_err->blocks_missed, tftp_err->blocks_received); > + *errstr = estrbuf; > + return -115; > + } > + > + return rc; > +} > diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h > index c94c94d..da743d3 100644 > --- a/lib/libnet/tftp.h > +++ b/lib/libnet/tftp.h > @@ -46,5 +46,7 @@ int tftp(filename_ip_t *fnip, unsigned char *buf, int len, tftp_err_t *err); > int32_t handle_tftp(int fd, uint8_t *, int32_t); > void handle_tftp_dun(uint8_t err_code); > int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, int len); > +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, > + const char **errstr, int *ecode); > > #endif >
On 24.05.2018 11:01, Alexey Kardashevskiy wrote: > On 19/5/18 1:45 am, Thomas Huth wrote: >> This way we can easily re-use the rc --> string translation in later >> patches. >> >> Reviewed-by: Greg Kurz <groug@kaod.org> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> lib/libnet/netload.c | 79 +++------------------------------------ >> lib/libnet/tftp.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> lib/libnet/tftp.h | 2 + >> 3 files changed, 111 insertions(+), 73 deletions(-) [...] >> diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c >> index b7121fe..ec8d631 100644 >> --- a/lib/libnet/tftp.c >> +++ b/lib/libnet/tftp.c >> @@ -690,3 +690,106 @@ int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, >> return 0; >> } >> } >> + >> +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, > > tftperr is not used. Oops, good catch, I missed to rename tftp_err to tftperr later in this function, so this accidentially used the "global" tftp_err of tftp.c :-( ... I'll fix it in v3. > >> + const char **errstr, int *ecode) >> +{ >> + static char estrbuf[80]; > > > >> + int dummy; >> + >> + if (!ecode) >> + ecode = &dummy; > > Nit: the other caller could also allocate this on the stack. Ok. >> + >> + if (rc == -1) { >> + *ecode = 0x3003; >> + *errstr = "unknown TFTP error"; >> + return -103; >> + } else if (rc == -2) { >> + *ecode = 0x3004; >> + sprintf(estrbuf, "TFTP buffer of %d bytes is too small for %s", >> + len, fnip->filename); >> + *errstr = estrbuf; >> + return -104; >> + } else if (rc == -3) { >> + *ecode = 0x3009; >> + sprintf(estrbuf, "file not found: %s", fnip->filename); > > estrbuf is 80 bytes, filename_ip::filename is 256. vsnprintf or increase > the estrbuf size or simply malloc it. I'd like to avoid malloc() here ... otherwise I'd have to put a strdup() around all the other strings and free() the memory in the caller again. Since most of the strings are static here, and the malloc() implementation in SLOF's libc is certainly not the best (it does not defragment the memory during free()), I think it's better to keep the "static" strings here. So I think I'll either increase the size of the buffer, or switch to snprintf (or strcpy + strncat) here instead. >> + *errstr = estrbuf; >> + return -108; >> + } else if (rc == -4) { >> + *ecode = 0x3010; >> + *errstr = "TFTP access violation"; >> + return -109; >> + } else if (rc == -5) { >> + *ecode = 0x3011; >> + *errstr = "illegal TFTP operation"; >> + return -110; >> + } else if (rc == -6) { >> + *ecode = 0x3012; >> + *errstr = "unknown TFTP transfer ID"; >> + return -111; >> + } else if (rc == -7) { >> + *ecode = 0x3013; >> + *errstr = "no such TFTP user"; >> + return -112; >> + } else if (rc == -8) { >> + *ecode = 0x3017; >> + *errstr = "TFTP blocksize negotiation failed"; >> + return -116; >> + } else if (rc == -9) { >> + *ecode = 0x3018; >> + *errstr = "file exceeds maximum TFTP transfer size"; >> + return -117; >> + } else if (rc <= -10 && rc >= -15) { >> + const char *icmp_err_str; >> + switch (rc) { >> + case -ICMP_NET_UNREACHABLE - 10: >> + icmp_err_str = "net unreachable"; >> + break; >> + case -ICMP_HOST_UNREACHABLE - 10: >> + icmp_err_str = "host unreachable"; >> + break; >> + case -ICMP_PROTOCOL_UNREACHABLE - 10: >> + icmp_err_str = "protocol unreachable"; >> + break; >> + case -ICMP_PORT_UNREACHABLE - 10: >> + icmp_err_str = "port unreachable"; >> + break; >> + case -ICMP_FRAGMENTATION_NEEDED - 10: >> + icmp_err_str = "fragmentation needed and DF set"; >> + break; >> + case -ICMP_SOURCE_ROUTE_FAILED - 10: >> + icmp_err_str = "source route failed"; >> + break; >> + default: >> + icmp_err_str = "UNKNOWN"; >> + break; >> + } >> + *ecode = 0x3005; >> + sprintf(estrbuf, "ICMP ERROR \"%s\"", icmp_err_str); >> + *errstr = estrbuf; >> + return -105; >> + } else if (rc == -40) { >> + *ecode = 0x3014; >> + sprintf(estrbuf, >> + "TFTP error occurred after %d bad packets received", >> + tftp_err->bad_tftp_packets); >> + *errstr = estrbuf; >> + return -113; >> + } else if (rc == -41) { >> + *ecode = 0x3015; >> + sprintf(estrbuf, >> + "TFTP error occurred after missing %d responses", >> + tftp_err->no_packets); >> + *errstr = estrbuf; >> + return -114; >> + } else if (rc == -42) { >> + *ecode = 0x3016; >> + sprintf(estrbuf, >> + "TFTP error missing block %d, expected block was %d", >> + tftp_err->blocks_missed, tftp_err->blocks_received); That should have been tftperr instead of tftp_err ... >> + *errstr = estrbuf; >> + return -115; >> + } >> + >> + return rc; >> +} >> diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h >> index c94c94d..da743d3 100644 >> --- a/lib/libnet/tftp.h >> +++ b/lib/libnet/tftp.h >> @@ -46,5 +46,7 @@ int tftp(filename_ip_t *fnip, unsigned char *buf, int len, tftp_err_t *err); >> int32_t handle_tftp(int fd, uint8_t *, int32_t); >> void handle_tftp_dun(uint8_t err_code); >> int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, int len); >> +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, >> + const char **errstr, int *ecode); >> >> #endif Thomas
diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c index 407d173..ba1008c 100644 --- a/lib/libnet/netload.c +++ b/lib/libnet/netload.c @@ -414,79 +414,12 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len) if (rc > 0) { printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, rc / 1024); - } else if (rc == -1) { - netload_error(0x3003, "unknown TFTP error"); - return -103; - } else if (rc == -2) { - netload_error(0x3004, "TFTP buffer of %d bytes " - "is too small for %s", - len, fnip->filename); - return -104; - } else if (rc == -3) { - netload_error(0x3009, "file not found: %s", - fnip->filename); - return -108; - } else if (rc == -4) { - netload_error(0x3010, "TFTP access violation"); - return -109; - } else if (rc == -5) { - netload_error(0x3011, "illegal TFTP operation"); - return -110; - } else if (rc == -6) { - netload_error(0x3012, "unknown TFTP transfer ID"); - return -111; - } else if (rc == -7) { - netload_error(0x3013, "no such TFTP user"); - return -112; - } else if (rc == -8) { - netload_error(0x3017, "TFTP blocksize negotiation failed"); - return -116; - } else if (rc == -9) { - netload_error(0x3018, "file exceeds maximum TFTP transfer size"); - return -117; - } else if (rc <= -10 && rc >= -15) { - const char *icmp_err_str; - switch (rc) { - case -ICMP_NET_UNREACHABLE - 10: - icmp_err_str = "net unreachable"; - break; - case -ICMP_HOST_UNREACHABLE - 10: - icmp_err_str = "host unreachable"; - break; - case -ICMP_PROTOCOL_UNREACHABLE - 10: - icmp_err_str = "protocol unreachable"; - break; - case -ICMP_PORT_UNREACHABLE - 10: - icmp_err_str = "port unreachable"; - break; - case -ICMP_FRAGMENTATION_NEEDED - 10: - icmp_err_str = "fragmentation needed and DF set"; - break; - case -ICMP_SOURCE_ROUTE_FAILED - 10: - icmp_err_str = "source route failed"; - break; - default: - icmp_err_str = " UNKNOWN"; - break; - } - netload_error(0x3005, "ICMP ERROR \"%s\"", icmp_err_str); - return -105; - } else if (rc == -40) { - netload_error(0x3014, "TFTP error occurred after " - "%d bad packets received", - tftp_err.bad_tftp_packets); - return -113; - } else if (rc == -41) { - netload_error(0x3015, "TFTP error occurred after " - "missing %d responses", - tftp_err.no_packets); - return -114; - } else if (rc == -42) { - netload_error(0x3016, "TFTP error missing block %d, " - "expected block was %d", - tftp_err.blocks_missed, - tftp_err.blocks_received); - return -115; + } else { + int ecode; + const char *errstr = NULL; + rc = tftp_get_error_info(fnip, &tftp_err, rc, &errstr, &ecode); + if (errstr) + netload_error(ecode, errstr); } return rc; diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c index b7121fe..ec8d631 100644 --- a/lib/libnet/tftp.c +++ b/lib/libnet/tftp.c @@ -690,3 +690,106 @@ int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, return 0; } } + +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, + const char **errstr, int *ecode) +{ + static char estrbuf[80]; + int dummy; + + if (!ecode) + ecode = &dummy; + + if (rc == -1) { + *ecode = 0x3003; + *errstr = "unknown TFTP error"; + return -103; + } else if (rc == -2) { + *ecode = 0x3004; + sprintf(estrbuf, "TFTP buffer of %d bytes is too small for %s", + len, fnip->filename); + *errstr = estrbuf; + return -104; + } else if (rc == -3) { + *ecode = 0x3009; + sprintf(estrbuf, "file not found: %s", fnip->filename); + *errstr = estrbuf; + return -108; + } else if (rc == -4) { + *ecode = 0x3010; + *errstr = "TFTP access violation"; + return -109; + } else if (rc == -5) { + *ecode = 0x3011; + *errstr = "illegal TFTP operation"; + return -110; + } else if (rc == -6) { + *ecode = 0x3012; + *errstr = "unknown TFTP transfer ID"; + return -111; + } else if (rc == -7) { + *ecode = 0x3013; + *errstr = "no such TFTP user"; + return -112; + } else if (rc == -8) { + *ecode = 0x3017; + *errstr = "TFTP blocksize negotiation failed"; + return -116; + } else if (rc == -9) { + *ecode = 0x3018; + *errstr = "file exceeds maximum TFTP transfer size"; + return -117; + } else if (rc <= -10 && rc >= -15) { + const char *icmp_err_str; + switch (rc) { + case -ICMP_NET_UNREACHABLE - 10: + icmp_err_str = "net unreachable"; + break; + case -ICMP_HOST_UNREACHABLE - 10: + icmp_err_str = "host unreachable"; + break; + case -ICMP_PROTOCOL_UNREACHABLE - 10: + icmp_err_str = "protocol unreachable"; + break; + case -ICMP_PORT_UNREACHABLE - 10: + icmp_err_str = "port unreachable"; + break; + case -ICMP_FRAGMENTATION_NEEDED - 10: + icmp_err_str = "fragmentation needed and DF set"; + break; + case -ICMP_SOURCE_ROUTE_FAILED - 10: + icmp_err_str = "source route failed"; + break; + default: + icmp_err_str = "UNKNOWN"; + break; + } + *ecode = 0x3005; + sprintf(estrbuf, "ICMP ERROR \"%s\"", icmp_err_str); + *errstr = estrbuf; + return -105; + } else if (rc == -40) { + *ecode = 0x3014; + sprintf(estrbuf, + "TFTP error occurred after %d bad packets received", + tftp_err->bad_tftp_packets); + *errstr = estrbuf; + return -113; + } else if (rc == -41) { + *ecode = 0x3015; + sprintf(estrbuf, + "TFTP error occurred after missing %d responses", + tftp_err->no_packets); + *errstr = estrbuf; + return -114; + } else if (rc == -42) { + *ecode = 0x3016; + sprintf(estrbuf, + "TFTP error missing block %d, expected block was %d", + tftp_err->blocks_missed, tftp_err->blocks_received); + *errstr = estrbuf; + return -115; + } + + return rc; +} diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h index c94c94d..da743d3 100644 --- a/lib/libnet/tftp.h +++ b/lib/libnet/tftp.h @@ -46,5 +46,7 @@ int tftp(filename_ip_t *fnip, unsigned char *buf, int len, tftp_err_t *err); int32_t handle_tftp(int fd, uint8_t *, int32_t); void handle_tftp_dun(uint8_t err_code); int parse_tftp_args(char buffer[], char *server_ip, char filename[], int fd, int len); +int tftp_get_error_info(filename_ip_t *fnip, tftp_err_t *tftperr, int rc, + const char **errstr, int *ecode); #endif