What is going on with stand/ofwboot?
Hauke Fath
what changes affected sparc/stand/ofwboot recently?
<snip>
--- dependall-sys ---
cc1: warnings being treated as errors
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c: In function
'net_mountroot_bootparams':
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c:141: warning:
implicit declaration of function 'bp_whoami'
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c:144: warning:
implicit declaration of function 'bp_getfile'
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c: In function
'net_mountroot_bootp':
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c:160: warning:
implicit declaration of function 'bootp'
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c: In function
'net_mountroot':
/public/netbsd-developer/sys/arch/sparc/stand/ofwboot/net.c:217: warning:
implicit declaration of function 'nfs_mount'
*** [net.o] Error code 1
nbmake: stopped in /public/netbsd-developer/sys/arch/sparc/stand/ofwboot
</snip>
It looks like function prototypes were made mandatory, and now the build is
crashing left and right, because functions and variable types have not been
exported properly.
I tried to fix a few occasions, but it looks like the ofwboot sources could
do with a bit of re-organizing from somebody in the know - it's more than
just the occasional missing prototype.
hauke
--
The ASCII Ribbon Campaign Hauke Fath
() No HTML/RTF in email Institut für Nachrichtentechnik
/\ No Word docs in email TU Darmstadt
Respect for open standards Ruf +49-6151-16-3281
--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-...@muc.de
Izumi Tsutsui
> It looks like function prototypes were made mandatory, and now the build is
> crashing left and right, because functions and variable types have not been
> exported properly.
Recently added -std=gnu99 in bsd.sys.mk requires function declarations.
> I tried to fix a few occasions, but it looks like the ofwboot sources could
> do with a bit of re-organizing from somebody in the know - it's more than
> just the occasional missing prototype.
net_open() in net.c stores a socket statically, so it looks net_tftp_bootp()
after net_open() doesn't have to return the socket in this case.
(probably busted of->f_devdata was not refered after that call fortunately)
Index: loadfile_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/stand/ofwboot/loadfile_machdep.c,v
retrieving revision 1.7
diff -u -p -r1.7 loadfile_machdep.c
--- loadfile_machdep.c 18 May 2009 11:39:30 -0000 1.7
+++ loadfile_machdep.c 19 May 2011 23:07:19 -0000
@@ -30,6 +30,7 @@
*/
#include <lib/libsa/stand.h>
+#include <lib/libkern/libkern.h>
#include <machine/pte.h>
#include <machine/cpu.h>
Index: net.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/stand/ofwboot/net.c,v
retrieving revision 1.5
diff -u -p -r1.5 net.c
--- net.c 7 May 2009 00:01:31 -0000 1.5
+++ net.c 19 May 2011 23:07:19 -0000
@@ -60,6 +60,9 @@
#include <lib/libsa/stand.h>
#include <lib/libsa/net.h>
#include <lib/libsa/netif.h>
+#include <lib/libsa/bootp.h>
+#include <lib/libsa/bootparam.h>
+#include <lib/libsa/nfs.h>
#include <lib/libkern/libkern.h>
@@ -176,14 +179,13 @@ net_mountroot_bootp(void)
}
int
-net_tftp_bootp(int **sock)
+net_tftp_bootp(struct of_dev *op)
{
net_mountroot_bootp();
if (myip.s_addr == 0)
return(ENOENT);
- *sock = &netdev_sock;
return (0);
}
Index: ofdev.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/stand/ofwboot/ofdev.c,v
retrieving revision 1.28
diff -u -p -r1.28 ofdev.c
--- ofdev.c 4 Apr 2010 21:49:15 -0000 1.28
+++ ofdev.c 19 May 2011 23:07:19 -0000
@@ -56,6 +56,7 @@
#include "ofdev.h"
#include "boot.h"
+#include "net.h"
extern char bootdev[];
extern bool root_fs_quickseekable;
@@ -545,7 +546,7 @@ open_again:
if (!strncmp(*file,"/tftp:",6)) {
*file += 6;
memcpy(&file_system[0], &file_system_tftp, sizeof file_system[0]);
- if (net_tftp_bootp(&of->f_devdata)) {
+ if (net_tftp_bootp(of->f_devdata)) {
net_close(&ofdev);
goto bad;
}
Index: promlib.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/stand/ofwboot/promlib.c,v
retrieving revision 1.2
diff -u -p -r1.2 promlib.c
--- promlib.c 28 Apr 2008 20:23:36 -0000 1.2
+++ promlib.c 19 May 2011 23:07:19 -0000
@@ -37,6 +37,9 @@
#include <sys/types.h>
#include <machine/promlib.h>
+#include <lib/libsa/stand.h>
+#include <lib/libkern/libkern.h>
+
#include "openfirm.h"
@@ -139,7 +142,7 @@ prom_init(void)
OF_initialize();
- memset(promops, 0, sizeof(promops));
+ memset(&promops, 0, sizeof(promops));
/* Access to boot arguments */
promops.po_bootpath = openfirmware_bootpath;
--- /dev/null 2011-05-20 07:51:06.000000000 +0900
+++ net.h 2011-05-20 08:08:31.000000000 +0900
@@ -0,0 +1,6 @@
+/* $NetBSD$ */
+
+int net_open(struct of_dev *);
+int net_close(struct of_dev *);
+int net_tftp_bootp(struct of_dev *);
+int net_mountroot(void);
---
Izumi Tsutsui
Havard Eidnes
>> do with a bit of re-organizing from somebody in the know - it's more than
>> just the occasional missing prototype.
>
> net_open() in net.c stores a socket statically, so it looks net_tftp_bootp()
> after net_open() doesn't have to return the socket in this case.
> (probably busted of->f_devdata was not refered after that call fortunately)
You probably saw that I made a commit which fixes the build, but
which doesn't re-do how net_tftp_bootp() behaves, and which now
requires an ugly cast (which I added).
Feel free to add your changes on top of the ones I made to make
it build again, that is, unless someone more authoritative has
any objections to the suggested changes.
BTW, won't your new net_tftp_bootp() get a warning for an unused
function argument?
And... this one:
> - memset(promops, 0, sizeof(promops));
> + memset(&promops, 0, sizeof(promops));
... looks like an ancient and somewhat unrelated bug(?)
Regards,
- Håvard
Izumi Tsutsui
> which doesn't re-do how net_tftp_bootp() behaves, and which now
> requires an ugly cast (which I added).
Well if you really want to fix buildable state I'd suggest
you'd follow minimum C style manner. Function declarations
added in net.h should also be included from a source where
they are actually implemented, so that you could notice
you also exported functions declared as static. It's also
better to add XXX comments if you don't know if the fix is
really correct.
> BTW, won't your new net_tftp_bootp() get a warning for an unused
> function argument?
Such unused function arguments are very common for consistent
API and we have not enabled such warnings in bsd.sys.mk:
---
.if ${WARNS} > 2
CFLAGS+= -Wcast-qual -Wwrite-strings
CFLAGS+= -Wextra -Wno-unused-parameter
---
then we don't have to add boring __unused attributes:
http://mail-index.NetBSD.org/source-changes/2006/10/12/0002.html
http://mail-index.NetBSD.org/source-changes/2006/11/16/0006.html
In this case, net_open() in some other implementation might
return a socket in the arg of struct of_dev as an opaque and
net_tftp_bootp() might require the value returned from net_open()
for network access. (nfs_mountroot() should also take it but
I didn't bother to change it)
> > - memset(promops, 0, sizeof(promops));
> > + memset(&promops, 0, sizeof(promops));
>
> ... looks like an ancient and somewhat unrelated bug(?)
WARNS often points out a real bug which should not be simply hidden?
---
Izumi Tsutsui
Christos Zoulas
Havard Eidnes <h...@NetBSD.org> wrote:
>>> I tried to fix a few occasions, but it looks like the ofwboot sources could
>>> do with a bit of re-organizing from somebody in the know - it's more than
>>> just the occasional missing prototype.
>>
>> net_open() in net.c stores a socket statically, so it looks net_tftp_bootp()
>> after net_open() doesn't have to return the socket in this case.
>> (probably busted of->f_devdata was not refered after that call fortunately)
>
>which doesn't re-do how net_tftp_bootp() behaves, and which now
>requires an ugly cast (which I added).
>
>Feel free to add your changes on top of the ones I made to make
>it build again, that is, unless someone more authoritative has
>any objections to the suggested changes.
>
>BTW, won't your new net_tftp_bootp() get a warning for an unused
>function argument?
>
>And... this one:
>
>> + memset(&promops, 0, sizeof(promops));
>
Well this is what adding -std=gnu99 has done. Unearth ancient bugs.
christos