Re: CVS commit: src/sys/dev

[Takeshi Nakayama]

>>> Robert Swindells  wrote

> 
> "Michael van Elst"  wrote:
> >Module Name:src
> >Committed By:   mlelstv
> >Date:   Wed Nov  1 19:34:46 UTC 2017
> >
> >Modified Files:
> >src/sys/dev: files.dev
> >src/sys/dev/ata: ata_raid_adaptec.c ata_raid_intel.c 
> > ata_raid_jmicron.c
> >ata_raid_nvidia.c ata_raid_promise.c ata_raid_via.c wd.c wdvar.h
> >
> >Log Message:
> >refactor wd and ataraid drivers to use common disk subroutines.
> 
> Should this work if wedges are not being used ?
> 
> Without any DKWEDGE_* options I now get:
> 
> ...
> opendisk: can't open dev wd0 (5)
> opendisk: can't open dev wd0 (5)
> opendisk: can't open dev wd0 (5)
> boot device: 
> root on wd0a dumps on wd0b
> vfs_mountroot: can't open root device
> cannot mount root, error = 5

errno 5 is EIO.

This is why we check WDF_LOADED is not set and !RAW_PART or !S_IFCHR
in wdopen().  WDF_LOADED is set only in wd_fistopen(), so we cannot
open disks unless open with RAW_PART and S_IFCHR in the first time.

Possible fix is remove useless check as follows or introduce
WDF_OPEN if we want to mimic the sd(4).

Index: wd.c
===
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.434
diff -u -d -r1.434 wd.c
--- wd.c1 Nov 2017 19:34:46 -   1.434
+++ wd.c3 Nov 2017 00:10:33 -
@@ -964,7 +964,7 @@
 {
struct wd_softc *wd;
struct dk_softc *dksc;
-   int unit, part, error;
+   int unit, error;
 
ATADEBUG_PRINT(("wdopen\n"), DEBUG_FUNCS);
unit = WDUNIT(dev);
@@ -976,20 +976,9 @@
if (! device_is_active(dksc->sc_dev))
return (ENODEV);
 
-   part = WDPART(dev);
-
if (wd->sc_capacity == 0)
return (ENODEV);
 
-   /*
-* If any partition is open, but the disk has been invalidated,
-* disallow further opens.
-*/
-   if ((wd->sc_flags & WDF_LOADED) == 0) {
-   if (part != RAW_PART || fmt != S_IFCHR)
-   return EIO;
-   }
-
error = dk_open(dksc, dev, flag, fmt, l);
 
return error;

-- Takeshi Nakayama

Re: CVS commit: src/sys/dev

[Robert Swindells]

"Michael van Elst"  wrote:
>Module Name:src
>Committed By:   mlelstv
>Date:   Wed Nov  1 19:34:46 UTC 2017
>
>Modified Files:
>src/sys/dev: files.dev
>src/sys/dev/ata: ata_raid_adaptec.c ata_raid_intel.c ata_raid_jmicron.c
>ata_raid_nvidia.c ata_raid_promise.c ata_raid_via.c wd.c wdvar.h
>
>Log Message:
>refactor wd and ataraid drivers to use common disk subroutines.

Should this work if wedges are not being used ?

Without any DKWEDGE_* options I now get:

...
opendisk: can't open dev wd0 (5)
opendisk: can't open dev wd0 (5)
opendisk: can't open dev wd0 (5)
boot device: 
root on wd0a dumps on wd0b
vfs_mountroot: can't open root device
cannot mount root, error = 5

Robert Swindells

Re: CVS commit: src/lib/libc/stdlib

[Joerg Sonnenberger]

On Thu, Nov 02, 2017 at 08:26:58PM +0100, Kamil Rytarowski wrote:
> On 02.11.2017 20:15, Joerg Sonnenberger wrote:
> > On Thu, Nov 02, 2017 at 08:15:01PM +0100, Joerg Sonnenberger wrote:
> >> On Thu, Nov 02, 2017 at 06:37:15PM +, Kamil Rytarowski wrote:
> >>> Module Name:  src
> >>> Committed By: kamil
> >>> Date: Thu Nov  2 18:37:15 UTC 2017
> >>>
> >>> Modified Files:
> >>>   src/lib/libc/stdlib: atexit.c
> >>>
> >>> Log Message:
> >>> Correct handling of __cxa_atexit(a,b,NULL) in libc
> >>
> >> I don't get it. This seems to be quite wrong.
> >>
> >>> Correct a bug that __cxa_atexit(x,y,NULL) is handled in the same way as
> >>> atexit(x) (which is simplified to __cxa_atexit(x,NULL,NULL).
> >>
> >> __cxa_atexit(x,y,NULL) is not invalid. Please revert this.
> > 
> > ...is invalid, of course.
> > 
> > Joerg
> > 
> 
> What's the rationale for being wrong?

The DSO handle is a required part of the (external) __cxa_atexit interface.
The atexit mapping is an implementation detail and not part of the public
interface. Doing it directly creates UB as it involves casting function
pointers between incompatible types.

Joerg

Re: CVS commit: src/lib/libc/stdlib

[Kamil Rytarowski]

On 02.11.2017 20:15, Joerg Sonnenberger wrote:
> On Thu, Nov 02, 2017 at 08:15:01PM +0100, Joerg Sonnenberger wrote:
>> On Thu, Nov 02, 2017 at 06:37:15PM +, Kamil Rytarowski wrote:
>>> Module Name:src
>>> Committed By:   kamil
>>> Date:   Thu Nov  2 18:37:15 UTC 2017
>>>
>>> Modified Files:
>>> src/lib/libc/stdlib: atexit.c
>>>
>>> Log Message:
>>> Correct handling of __cxa_atexit(a,b,NULL) in libc
>>
>> I don't get it. This seems to be quite wrong.
>>
>>> Correct a bug that __cxa_atexit(x,y,NULL) is handled in the same way as
>>> atexit(x) (which is simplified to __cxa_atexit(x,NULL,NULL).
>>
>> __cxa_atexit(x,y,NULL) is not invalid. Please revert this.
> 
> ...is invalid, of course.
> 
> Joerg
> 

What's the rationale for being wrong?



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libc/stdlib

[Joerg Sonnenberger]

On Thu, Nov 02, 2017 at 08:15:01PM +0100, Joerg Sonnenberger wrote:
> On Thu, Nov 02, 2017 at 06:37:15PM +, Kamil Rytarowski wrote:
> > Module Name:src
> > Committed By:   kamil
> > Date:   Thu Nov  2 18:37:15 UTC 2017
> > 
> > Modified Files:
> > src/lib/libc/stdlib: atexit.c
> > 
> > Log Message:
> > Correct handling of __cxa_atexit(a,b,NULL) in libc
> 
> I don't get it. This seems to be quite wrong.
> 
> > Correct a bug that __cxa_atexit(x,y,NULL) is handled in the same way as
> > atexit(x) (which is simplified to __cxa_atexit(x,NULL,NULL).
> 
> __cxa_atexit(x,y,NULL) is not invalid. Please revert this.

...is invalid, of course.

Joerg

Re: CVS commit: src/lib/libc/stdlib

[Joerg Sonnenberger]

On Thu, Nov 02, 2017 at 06:37:15PM +, Kamil Rytarowski wrote:
> Module Name:  src
> Committed By: kamil
> Date: Thu Nov  2 18:37:15 UTC 2017
> 
> Modified Files:
>   src/lib/libc/stdlib: atexit.c
> 
> Log Message:
> Correct handling of __cxa_atexit(a,b,NULL) in libc

I don't get it. This seems to be quite wrong.

> Correct a bug that __cxa_atexit(x,y,NULL) is handled in the same way as
> atexit(x) (which is simplified to __cxa_atexit(x,NULL,NULL).

__cxa_atexit(x,y,NULL) is not invalid. Please revert this.

Joerg