Chris' recent plevent change has broken the OpenVMS build. I guess the OpenVMS specific changes were coded in a confusing way, but OpenVMS builds with both VMS and XP_UNIX defined. Anyway, I have a patch and will attach next. Since this is platform specific I'd like to get it in to M0.81 if poss.
Is that patch backwards or something?
No, its the right way. I just need to add the two declarations to the VMS leg and then two lines of zeroizing to another VMS leg. I moved the first VMS ifdef inside of the XP_UNIX ifdef to make it obvious that VMS is a subset of XP_UNIX. That's why it looks a little funny.
Oh, OK. I'd much rather that you did something like: #ifdef VMS (..) #endif /* VMS */ #ifdef XP_UNIX (..) #endif I really don't like nested ifs. They are hard to read and unless there's a really good reason I'd rather not see them used.
That's the ifdef structure I was using in plevent.c, and it bit me because you didn't realize that I needed all the XP_UNIX changes. I thought nested ifdef's would make that fact obvious (at the expense of a little readability, I admit). But if you think its too gross I'll do it the old way.
Status: NEW → ASSIGNED
Another reason for that particular set of ifdefs being nested is because there's a certain amount of the code that's common to VMS and XP_UNIX, and I wanted to emphasis this (as well as hopefully not breaking again the next time this code is changed). So instead of: #ifdef VMS a1 b c #endif /* VMS */ #ifdef XP_UNIX a2 b c #endif I have: #ifdef XP_UNIX #ifdef VMS a1 #else a2 #endif b c #endif /* XP_UNIX */ Please let me know this morning which you prefer, as I really want to get this in before we branch.
In the future try and stay away from netsted comments, that's all. Get this in if you've got reviews from danm. r=blizzard
r=danm. sorry for the bustage. (I don't have sooper review status. Chris, wanna upgrade your r?) The VMS ifdeffage in this file doesn't make it clear that it's dependent on XP_UNIX. Might want to add a comment while you're in there; maybe it'll help.
Yeah, you can use my r= any way you want. I'm flexible.
Checked in before the M0.81 branch. rev 1.19.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.