tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Changing the return value of xxx_attach() from void to int.



 Hi, all.

 As you know, the return value of device driver's attach function is void.
I've thought that we should change it to int for many years. I believe I'm
not the only person.

 xxx_attach() may fail the following cases:

	got unexpected behavior.

	resource allocation error.

	driver is really broken.

	some others.

 xxx_attach() is void, so the caller can't know the fail. It makes some
problems:

	a) The OS may touch the broken device while the OS is running.
	   It may causes a panic.

	b) The shutdown sequence calls xxx_shutdown() even if
	   it's not really attached. It may causes panic. We have met
	   this bugs many times.

	c) To prevent b), we have added extra code into xxx_shutdown().
	   It wastes our time and the code become big and complex.

	d) Resource leak. For example, A device_t structure is allocated
	   by caller side.

 If we change the return value to int or any other type's value, we can
do the following things.

	a) Don't register failed device to avoid problem.

	b) If we don't do a), we can add code to not to call xxx_shutdown()
	   for broken devices instead.

	c) We can add the code to fallback to lower priority device driver
	  like ukphy(4), ugen(4) or others.

	d) (add some other good features?)

 What do you think about this change? If you're OK, I'll change _ALL_ device
drivers' attach function first by the following way:

	0) Change return value to int.

	1) Change "return;" to "return 0;" or "return -1;"
	   (or bool value or Exxx. See below)

	2) I won't modify the caller side for the time being.

In this way, it won't break anything because the caller doesn't check the
value. Even if I mistakenly modified the return value and we add code to
check the return value, it won't be a big problem because almost all drivers
don't go into the failure paths. So, I think it's not required to use new
CFATTACH_XXXX() because it makes both the driver and the caller side be
complex.

 Now I'm wondering if I should return "-1" or Exxx. FreeBSD returns Exxx
(e.g. ENOMEN, EIO, ENXIO, etc.). I don't know which one is better.

	Changing to -1 is easier than Exxx because it's not required
	to wonder what Exxx I should choose.

	Changing to Exxx may be good if we check the error code and
	do something depend on the value though I can't imagine anything now.

	-1 and Exxx can be mixed because both are int. We can detect
	error with "if (rv != 0)" even if it's mixed.

 Any objection, advice or idea?

 Thanks.

--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)



Home | Main Index | Thread Index | Old Index