On Dec 5, 1:57pm, [email protected] (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
