Current-Users archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Crash on -current on i386 (NOT amd64)
On Dec 5, 1:57pm, jnemeth%cue.bc.ca@localhost (John Nemeth) wrote:
-- Subject: Re: Crash on -current on i386 (NOT amd64)
| I don't think there was a backtrace. I saw your comment on
| chat that you had fixed the lseek(SEEK_END) issue, and was curious
| what it was, so I looked for the commit, and spotted the bug. I
| then saw Paul's message about his crash about mounting a CD and
| noted your new DIAGNOSTIC message about DIOCGMEDIASIZE failing in
| his output, which hinted that it was your patch. At first, I was
| going to suggest that he revert it, but then decided that I could
| fix it (I don't know anything about file system code, so normally
| don't go anywhere near it).
|
| The issues goes like this. The original code in pseudo code
| format (with irrelevant details omitted goes like this):
|
| 1) ioctl(DIOCGPART)
| 2) if successful
| 3) use struct partinfo to determine disk size
| 4) call uvm_vnp_setsize(disk size)
|
| Your new code did:
|
| 1) ioctl(DIOCGMEDIASIZE)
| 2) if failure
| 3) diagnostic(DIOCGMEDIASIZE failed)
| 4) ioctl(DIOCGPART)
| 5) use struct partinfo to determine disk size
| 6) if either ioctl was successful
| 7) call uvm_vnp_setsize(disk size)
|
| Note that at step 5, you were dereferencing two pointers in struct
| partinfo regardless of whether or not the ioctl(DIOCGPART) was
| successful, which is what initializes the struct partinfo. I just
| did the obvious and inserted step 4.5, "if successful", to avoid
| dereferencing NULL pointers.
Yes, I know.
| The implication of all this is that the CD driver in question
| doesn't support either DIOCGMEDIASIZE or DIOCGPART. Thus for the
| CD driver, there is no functional difference between the original
| code and your new code (with my fix).
Hmm, there should be a backtrace though and we could fix the driver...
Actually I want to get rid of most of the disklabel code (leaving
behind the DEFLABEL code for a new 64 bit struct.
christos
Home |
Main Index |
Thread Index |
Old Index