[syslinux] Makefile race condition with parallel make
Josh Triplett
josh at joshtriplett.org
Thu Mar 28 13:10:52 PDT 2013
On Thu, Mar 28, 2013 at 12:12:44PM -0700, H. Peter Anvin wrote:
> I think this patch should disentangle the dependencies by avoiding the
> need for IS_LPXELINUX.
>
> Please let me know if it works for you.
I can't reliably reproduce the race in the unpatched version (I've only
ever seen it once), but this builds for me, and by inspection it
definitely fixes the race I observed.
Reviewed-by: Josh Triplett <josh at joshtriplett.org>
> diff --git a/core/Makefile b/core/Makefile
> index 1890b24..59a61eb 100644
> --- a/core/Makefile
> +++ b/core/Makefile
> @@ -138,15 +138,12 @@ libpxelinux.a: rawcon.o $(PXELINUX_OBJS)
> rm -f $@
> $(AR) cq $@ $^
> $(RANLIB) $@
> - rm $(PXELINUX_OBJS)
>
> # LwIP network stack
> -liblpxelinux.a: rawcon.o
> - $(MAKE) CFLAGS="$(CFLAGS) -DIS_LPXELINUX" $(LPXELINUX_OBJS)
> +liblpxelinux.a: rawcon.o $(LPXELINUX_OBJS)
> rm -f $@
> - $(AR) cq $@ $^ $(LPXELINUX_OBJS)
> + $(AR) cq $@ $^
> $(RANLIB) $@
> - rm $(LPXELINUX_OBJS)
>
> libldlinux.a: plaincon.o
> rm -f $@
> diff --git a/core/fs/pxe/core.c b/core/fs/pxe/core.c
> index 9246f66..e330ba8 100644
> --- a/core/fs/pxe/core.c
> +++ b/core/fs/pxe/core.c
> @@ -6,6 +6,13 @@
> #include <net.h>
> #include "pxe.h"
>
> +const struct url_scheme url_schemes[] = {
> + { "tftp", tftp_open, 0 },
> + { "http", http_open, O_DIRECTORY },
> + { "ftp", ftp_open, O_DIRECTORY },
> + { NULL, NULL, 0 },
> +};
> +
> /**
> * Open a socket
> *
> @@ -16,7 +23,7 @@
> */
> int net_core_open(struct pxe_pvt_inode *socket, enum net_core_proto proto)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_lwip *priv = &socket->net.lwip;
> enum netconn_type type;
> int err;
>
> @@ -53,7 +60,7 @@ int net_core_open(struct pxe_pvt_inode *socket, enum net_core_proto proto)
> */
> void net_core_close(struct pxe_pvt_inode *socket)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_lwip *priv = &socket->net.lwip;
>
> if (priv->conn) {
> netconn_delete(priv->conn);
> @@ -71,7 +78,7 @@ void net_core_close(struct pxe_pvt_inode *socket)
> void net_core_connect(struct pxe_pvt_inode *socket, uint32_t ip,
> uint16_t port)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_lwip *priv = &socket->net.lwip;
> struct ip_addr addr;
>
> addr.addr = ip;
> @@ -85,7 +92,7 @@ void net_core_connect(struct pxe_pvt_inode *socket, uint32_t ip,
> */
> void net_core_disconnect(struct pxe_pvt_inode *socket)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_lwip *priv = &socket->net.lwip;
> netconn_disconnect(priv->conn);
> }
>
> @@ -102,7 +109,7 @@ void net_core_disconnect(struct pxe_pvt_inode *socket)
> int net_core_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len,
> uint32_t *src_ip, uint16_t *src_port)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_lwip *priv = &socket->net.lwip;
> struct netbuf *nbuf;
> u16_t nbuf_len;
> int err;
> @@ -138,7 +145,7 @@ int net_core_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len,
> */
> void net_core_send(struct pxe_pvt_inode *socket, const void *data, size_t len)
> {
> - struct netconn *conn = socket->private.conn;
> + struct netconn *conn = socket->net.lwip.conn;
> struct netbuf *nbuf;
> void *pbuf;
> int err;
> diff --git a/core/fs/pxe/ftp.c b/core/fs/pxe/ftp.c
> index 5d5a6f0..c2d155a 100644
> --- a/core/fs/pxe/ftp.c
> +++ b/core/fs/pxe/ftp.c
> @@ -69,7 +69,7 @@ static int ftp_cmd_response(struct inode *inode, const char *cmd,
> *q++ = '\n';
> cmd_len += 2;
>
> - err = netconn_write(socket->private.conn, cmd_buf, cmd_len, NETCONN_COPY);
> + err = netconn_write(socket->net.lwip.conn, cmd_buf, cmd_len, NETCONN_COPY);
> if (err)
> return -1;
> }
> @@ -166,7 +166,7 @@ static void ftp_close_file(struct inode *inode)
> int resp;
>
> ctlsock = socket->ctl ? PVT(socket->ctl) : NULL;
> - if (ctlsock->private.conn) {
> + if (ctlsock->net.lwip.conn) {
> resp = ftp_cmd_response(socket->ctl, "QUIT", NULL, NULL, NULL);
> while (resp == 226) {
> resp = ftp_cmd_response(socket->ctl, NULL, NULL, NULL, NULL);
> @@ -209,11 +209,11 @@ void ftp_open(struct url_info *url, int flags, struct inode *inode,
> return;
> ctlsock = PVT(socket->ctl);
> ctlsock->ops = &tcp_conn_ops; /* The control connection is just TCP */
> - ctlsock->private.conn = netconn_new(NETCONN_TCP);
> - if (!ctlsock->private.conn)
> + ctlsock->net.lwip.conn = netconn_new(NETCONN_TCP);
> + if (!ctlsock->net.lwip.conn)
> goto err_free;
> addr.addr = url->ip;
> - err = netconn_connect(ctlsock->private.conn, &addr, url->port);
> + err = netconn_connect(ctlsock->net.lwip.conn, &addr, url->port);
> if (err)
> goto err_delete;
>
> @@ -248,10 +248,10 @@ void ftp_open(struct url_info *url, int flags, struct inode *inode,
> if (resp != 227 || pasv_bytes != 6)
> goto err_disconnect;
>
> - socket->private.conn = netconn_new(NETCONN_TCP);
> - if (!socket->private.conn)
> + socket->net.lwip.conn = netconn_new(NETCONN_TCP);
> + if (!socket->net.lwip.conn)
> goto err_disconnect;
> - err = netconn_connect(socket->private.conn, (struct ip_addr *)&pasv_data[0],
> + err = netconn_connect(socket->net.lwip.conn, (struct ip_addr *)&pasv_data[0],
> ntohs(*(uint16_t *)&pasv_data[4]));
> if (err)
> goto err_disconnect;
> @@ -266,15 +266,15 @@ void ftp_open(struct url_info *url, int flags, struct inode *inode,
> return; /* Sucess! */
>
> err_disconnect:
> - if (ctlsock->private.conn)
> - netconn_write(ctlsock->private.conn, "QUIT\r\n", 6, NETCONN_NOCOPY);
> - if (socket->private.conn)
> - netconn_delete(socket->private.conn);
> - if (ctlsock->private.buf)
> - netbuf_delete(ctlsock->private.buf);
> + if (ctlsock->net.lwip.conn)
> + netconn_write(ctlsock->net.lwip.conn, "QUIT\r\n", 6, NETCONN_NOCOPY);
> + if (socket->net.lwip.conn)
> + netconn_delete(socket->net.lwip.conn);
> + if (ctlsock->net.lwip.buf)
> + netbuf_delete(ctlsock->net.lwip.buf);
> err_delete:
> - if (ctlsock->private.conn)
> - netconn_delete(ctlsock->private.conn);
> + if (ctlsock->net.lwip.conn)
> + netconn_delete(ctlsock->net.lwip.conn);
> err_free:
> free_socket(socket->ctl);
> }
> diff --git a/core/fs/pxe/http.c b/core/fs/pxe/http.c
> index 9bf0c92..94f1059 100644
> --- a/core/fs/pxe/http.c
> +++ b/core/fs/pxe/http.c
> @@ -191,8 +191,8 @@ void http_open(struct url_info *url, int flags, struct inode *inode,
> inode->size = content_length = -1;
>
> /* Start the http connection */
> - socket->private.conn = netconn_new(NETCONN_TCP);
> - if (!socket->private.conn) {
> + socket->net.lwip.conn = netconn_new(NETCONN_TCP);
> + if (!socket->net.lwip.conn) {
> printf("netconn_new failed\n");
> return;
> }
> @@ -201,7 +201,7 @@ void http_open(struct url_info *url, int flags, struct inode *inode,
> if (!url->port)
> url->port = HTTP_PORT;
>
> - err = netconn_connect(socket->private.conn, &addr, url->port);
> + err = netconn_connect(socket->net.lwip.conn, &addr, url->port);
> if (err) {
> printf("netconn_connect error %d\n", err);
> goto fail;
> @@ -225,7 +225,7 @@ void http_open(struct url_info *url, int flags, struct inode *inode,
> if (header_bytes >= header_len)
> goto fail; /* Buffer overflow */
>
> - err = netconn_write(socket->private.conn, header_buf,
> + err = netconn_write(socket->net.lwip.conn, header_buf,
> header_bytes, NETCONN_NOCOPY);
> if (err) {
> printf("netconn_write error %d\n", err);
> diff --git a/core/fs/pxe/pxe.c b/core/fs/pxe/pxe.c
> index 4d694a1..3f68e96 100644
> --- a/core/fs/pxe/pxe.c
> +++ b/core/fs/pxe/pxe.c
> @@ -29,20 +29,6 @@ static bool has_gpxe;
> static uint32_t gpxe_funcs;
> bool have_uuid = false;
>
> -static struct url_scheme {
> - const char *name;
> - void (*open)(struct url_info *, int, struct inode *, const char **);
> - int ok_flags;
> -} url_schemes[] = {
> - { "tftp", tftp_open, 0 },
> -#ifdef IS_LPXELINUX
> - { "http", http_open, O_DIRECTORY },
> - { "ftp", ftp_open, O_DIRECTORY },
> -#endif
> - { NULL, NULL, 0 },
> -};
> -#define OK_FLAGS_MASK (O_DIRECTORY|O_WRONLY)
> -
> /*
> * Allocate a local UDP port structure and assign it a local port number.
> * Return the inode pointer if success, or null if failure
> diff --git a/core/fs/pxe/pxe.h b/core/fs/pxe/pxe.h
> index 2d4be3b..68d4e3e 100644
> --- a/core/fs/pxe/pxe.h
> +++ b/core/fs/pxe/pxe.h
> @@ -21,6 +21,7 @@
> #define PXE_H
>
> #include <syslinux/pxe_api.h>
> +#include <fcntl.h> /* For OK_FLAGS_MASK */
> #include "fs.h" /* Mostly for FILENAME_MAX */
>
> /*
> @@ -114,19 +115,19 @@ struct pxe_conn_ops {
> int (*readdir)(struct inode *inode, struct dirent *dirent);
> };
>
> -struct net_private {
> -#ifdef IS_LPXELINUX
> - struct netconn *conn; /* lwip network connection */
> - struct netbuf *buf; /* lwip cached buffer */
> -#else
> - uint16_t tftp_localport; /* Local port number (0=not in use) */
> - uint32_t tftp_remoteip; /* Remote IP address (0 = disconnected) */
> -#endif
> -
> +union net_private {
> + struct net_private_lwip {
> + struct netconn *conn; /* lwip network connection */
> + struct netbuf *buf; /* lwip cached buffer */
> + } lwip;
> + struct net_private_tftp {
> + uint32_t remoteip; /* Remote IP address (0 = disconnected) */
> + uint16_t localport; /* Local port number (0=not in use) */
> + } tftp;
> };
>
> struct pxe_pvt_inode {
> - struct net_private private; /* Network stack private data */
> + union net_private net; /* Network stack private data */
> uint16_t tftp_remoteport; /* Remote port number */
> uint32_t tftp_filepos; /* bytes downloaded (including buffer) */
> uint32_t tftp_blksize; /* Block size for this connection(*) */
> @@ -185,6 +186,17 @@ extern bool have_uuid;
> extern uint8_t uuid_type;
> extern uint8_t uuid[];
>
> +struct url_info;
> +struct url_scheme {
> + const char *name;
> + void (*open)(struct url_info *, int, struct inode *, const char **);
> + int ok_flags;
> +};
> +/* Flags which can be specified in url_scheme.ok_flags */
> +#define OK_FLAGS_MASK (O_DIRECTORY|O_WRONLY)
> +
> +extern const struct url_scheme url_schemes[];
> +
> /*
> * Compute the suitable gateway for a specific route -- too many
> * vendor PXE stacks don't do this correctly...
> diff --git a/core/fs/pxe/tcp.c b/core/fs/pxe/tcp.c
> index 528fbce..9076197 100644
> --- a/core/fs/pxe/tcp.c
> +++ b/core/fs/pxe/tcp.c
> @@ -25,13 +25,13 @@ void tcp_close_file(struct inode *inode)
> {
> struct pxe_pvt_inode *socket = PVT(inode);
>
> - if (socket->private.conn) {
> - netconn_delete(socket->private.conn);
> - socket->private.conn = NULL;
> + if (socket->net.lwip.conn) {
> + netconn_delete(socket->net.lwip.conn);
> + socket->net.lwip.conn = NULL;
> }
> - if (socket->private.buf) {
> - netbuf_delete(socket->private.buf);
> - socket->private.buf = NULL;
> + if (socket->net.lwip.buf) {
> + netbuf_delete(socket->net.lwip.buf);
> + socket->net.lwip.buf = NULL;
> }
> }
>
> @@ -43,16 +43,16 @@ void tcp_fill_buffer(struct inode *inode)
> err_t err;
>
> /* Clean up or advance an inuse netbuf */
> - if (socket->private.buf) {
> - if (netbuf_next(socket->private.buf) < 0) {
> - netbuf_delete(socket->private.buf);
> - socket->private.buf = NULL;
> + if (socket->net.lwip.buf) {
> + if (netbuf_next(socket->net.lwip.buf) < 0) {
> + netbuf_delete(socket->net.lwip.buf);
> + socket->net.lwip.buf = NULL;
> }
> }
> /* If needed get a new netbuf */
> - if (!socket->private.buf) {
> - err = netconn_recv(socket->private.conn, &(socket->private.buf));
> - if (!socket->private.buf || err) {
> + if (!socket->net.lwip.buf) {
> + err = netconn_recv(socket->net.lwip.conn, &(socket->net.lwip.buf));
> + if (!socket->net.lwip.buf || err) {
> socket->tftp_goteof = 1;
> if (inode->size == -1)
> inode->size = socket->tftp_filepos;
> @@ -61,7 +61,7 @@ void tcp_fill_buffer(struct inode *inode)
> }
> }
> /* Report the current fragment of the netbuf */
> - err = netbuf_data(socket->private.buf, &data, &len);
> + err = netbuf_data(socket->net.lwip.buf, &data, &len);
> if (err) {
> printf("netbuf_data err: %d\n", err);
> kaboom();
> diff --git a/core/legacynet/core.c b/core/legacynet/core.c
> index 2163c69..848410c 100644
> --- a/core/legacynet/core.c
> +++ b/core/legacynet/core.c
> @@ -11,6 +11,11 @@ static __lowmem char packet_buf[PKTBUF_SIZE] __aligned(16);
> extern uint16_t get_port(void);
> extern void free_port(uint16_t);
>
> +const struct url_scheme url_schemes[] = {
> + { "tftp", tftp_open, 0 },
> + { NULL, NULL, 0 }
> +};
> +
> /**
> * Open a socket
> *
> @@ -22,14 +27,14 @@ extern void free_port(uint16_t);
> int net_core_open(struct pxe_pvt_inode *socket __unused,
> enum net_core_proto proto)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_tftp *priv = &socket->net.tftp;
>
> /* The legacy stack only supports UDP */
> if (proto != NET_CORE_UDP)
> return -1;
>
> /* Allocate local UDP port number */
> - priv->tftp_localport = get_port();
> + priv->localport = get_port();
>
> return 0;
> }
> @@ -41,10 +46,10 @@ int net_core_open(struct pxe_pvt_inode *socket __unused,
> */
> void net_core_close(struct pxe_pvt_inode *socket)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_tftp *priv = &socket->net.tftp;
>
> - if (priv->tftp_localport)
> - free_port(priv->tftp_localport);
> + if (priv->localport)
> + free_port(priv->localport);
> }
>
> /**
> @@ -57,10 +62,10 @@ void net_core_close(struct pxe_pvt_inode *socket)
> void net_core_connect(struct pxe_pvt_inode *socket, uint32_t ip,
> uint16_t port)
> {
> - struct net_private *priv = &socket->private;
> + struct net_private_tftp *priv = &socket->net.tftp;
>
> socket->tftp_remoteport = htons(port);
> - priv->tftp_remoteip = ip;
> + priv->remoteip = ip;
>
> }
>
> @@ -87,7 +92,7 @@ int net_core_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len,
> uint32_t *src_ip, uint16_t *src_port)
> {
> static __lowmem struct s_PXENV_UDP_READ udp_read;
> - struct net_private *priv = &socket->private;
> + struct net_private_tftp *priv = &socket->net.tftp;
> uint16_t bytes;
> int err;
>
> @@ -95,7 +100,7 @@ int net_core_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len,
> udp_read.buffer = FAR_PTR(packet_buf);
> udp_read.buffer_size = PKTBUF_SIZE;
> udp_read.dest_ip = IPInfo.myip;
> - udp_read.d_port = priv->tftp_localport;
> + udp_read.d_port = priv->localport;
>
> err = pxe_call(PXENV_UDP_READ, &udp_read);
> if (err)
> @@ -124,7 +129,7 @@ int net_core_recv(struct pxe_pvt_inode *socket, void *buf, uint16_t *buf_len,
> void net_core_send(struct pxe_pvt_inode *socket, const void *data, size_t len)
> {
> static __lowmem struct s_PXENV_UDP_WRITE udp_write;
> - struct net_private *priv = &socket->private;
> + struct net_private_tftp *priv = &socket->net.tftp;
> void *lbuf;
> uint16_t tid;
>
> @@ -134,9 +139,9 @@ void net_core_send(struct pxe_pvt_inode *socket, const void *data, size_t len)
>
> memcpy(lbuf, data, len);
>
> - tid = priv->tftp_localport; /* TID(local port No) */
> + tid = priv->localport; /* TID(local port No) */
> udp_write.buffer = FAR_PTR(lbuf);
> - udp_write.ip = priv->tftp_remoteip;
> + udp_write.ip = priv->remoteip;
> udp_write.gw = gateway(udp_write.ip);
> udp_write.src_port = tid;
> udp_write.dst_port = socket->tftp_remoteport;
More information about the Syslinux
mailing list