|
6174 posted:What I find interesting from that links.org post is a lot of people are railing on Debian for not contributing the patch upstream, or at least asking upstream if it was an issue. Yet in the comments there is a link to a openssl-dev thread (http://marc.info/?t=114651088900003&r=1&w=2) where they did precisely that and no one mentioned it was a bad idea, and one of the responders, Ulf Möller (listed as a dev team member on http://openssl.org/about/) said he was in favor of removing the calls. Seems to me that at least some of the blame for this should be on OpenSSL. I agree. This whole thing is a (drat annoying) lesson in how to behave in a public programming ecosystem, IMO.
|
# ? May 13, 2008 21:04 |
|
|
# ? May 16, 2024 18:15 |
|
This was really awesome.code:
code:
|
# ? May 14, 2008 01:16 |
|
It's gonna be even more awesome when two threads call someRandomClass() at the same time and end up setting numberOfPersons incorrectly due to a lack of synchronization.
|
# ? May 14, 2008 03:43 |
|
forelle posted:I have a real objection to the "It can never be NULL here so I'm not going to check for it." idea. Adding checks tells me (the maintainer) that these pointers should never be NULL and if they are, something is hosed up. I completely disagree with this, unless you mean checks that only exist when compiling a debug build such as an assert. If by context your data can never be in a particular state, checking for that state in a release build every time that bit of code is run is just silly. For example, if I'm writing an iterator for something like a linked list, I'm not going to check at every increment or decrement if the pointer I'm dereferencing is NULL. Write proper unit tests rather than flooding your applications with redundant checks that you often can't do anything appropriate about if they fail anyway. If you really want to just inform the reader of the code that the data can't be NULL there as you claim, then use a comment rather than introducing a runtime check.
|
# ? May 14, 2008 07:13 |
|
That Turkey Story posted:I completely disagree with this, unless you mean checks that only exist when compiling a debug build such as an assert. If by context your data can never be in a particular state, checking for that state in a release build every time that bit of code is run is just silly. For example, if I'm writing an iterator for something like a linked list, I'm not going to check at every increment or decrement if the pointer I'm dereferencing is NULL. Write proper unit tests rather than flooding your applications with redundant checks that you often can't do anything appropriate about if they fail anyway. If you really want to just inform the reader of the code that the data can't be NULL there as you claim, then use a comment rather than introducing a runtime check. Why? If you're about to dereference a pointer in a dynamically-allocated structure, then there's a very good chance you've lost memory locality and are about to stall on memory access anyway, so the check is essentially free. It inflates code size slightly, so I wouldn't do it inside a tight loop, but I'd be trying not to traverse large structures in a tight loop anyway.
|
# ? May 14, 2008 07:17 |
|
ShoulderDaemon posted:Why? If you're about to dereference a pointer in a dynamically-allocated structure, then there's a very good chance you've lost memory locality and are about to stall on memory access anyway, so the check is essentially free. It inflates code size slightly, so I wouldn't do it inside a tight loop, but I'd be trying not to traverse large structures in a tight loop anyway. Because as expensive as LHSes are, mispredicted branches can still hurt quite a bit on modern architecture. Remember, the fastest and least buggy code is the code that's not run, or ideally never written.
|
# ? May 14, 2008 09:57 |
|
tef posted:It turns out I'm talking poo poo! 1) Relying on specific, non-guaranteed behaviour from uninitialised memory. 2) Knowing that you get specific warnings about sections of code and failing to comment them appropriately. 3) Making a change to some code you don't fully understand without getting in touch with either the original author, or someone who could explain why it's doing what it is. If you ask me, then the original OpenSSL devs are just as much to blame for that clusterfuck as the guy who put in an incorrect 'fix' for the problem.
|
# ? May 14, 2008 10:13 |
|
TSDK posted:It seems to me then that there were 3 WTFs: Interestingly the first fix posted (mentioned in the bug as not the right way to do it), would have been much less damaging. As for the idea of using uninitialised data as a source of entropy, it's risky as it's possible an attacker could control that data. The real WTF is that, from looking at the code, that buffer should have been filled from /dev/urandom and thus never triggered the warnings. It seems that it would only have used uninitialised data if /dev/urandom was unavailable.
|
# ? May 14, 2008 11:35 |
|
That Turkey Story posted:I completely disagree with this, unless you mean checks that only exist when compiling a debug build such as an assert. If by context your data can never be in a particular state, checking for that state in a release build every time that bit of code is run is just silly. For example, if I'm writing an iterator for something like a linked list, I'm not going to check at every increment or decrement if the pointer I'm dereferencing is NULL. Write proper unit tests rather than flooding your applications with redundant checks that you often can't do anything appropriate about if they fail anyway. If you really want to just inform the reader of the code that the data can't be NULL there as you claim, then use a comment rather than introducing a runtime check. Meh, I guess I'm just a very defensive programmer. I don't really trust anyone, most of all myself. I don't write or ship perfect code and if I can get meaningful messages back from the field ("NULL Pointer detected in blah, line blah") I'm a happy man. Even if it crashes, that message alone can be worth thousands of pounds in reduced debugging time. I've never seen an instance where checking for NULL caused a bug. If I ever find that my application is running too slowly because I'm checking for NULLs I'll remove them where needed. Strangely I've never got to the point where that is my bottle neck. But, this is derailing this thread and should probably be dropped.
|
# ? May 14, 2008 11:38 |
|
TSDK posted:3) Making a change to some code you don't fully understand without getting in touch with either the original author, or someone who could explain why it's doing what it is. http://marc.info/?l=openssl-dev&m=114651085826293&w=2 An open ssl dev wrote in reply posted:Kurt Roeckx schrieb:
|
# ? May 14, 2008 11:45 |
I don't know if this counts as a coding horror, but it certainly makes me cry.code:
And imagine my surprise when working with people who use Oracle who don't even understand the most basic tenets of it (I'll give you a hint: it was none)
|
|
# ? May 14, 2008 14:13 |
|
shopvac4christ posted:I don't know if this counts as a coding horror, but it certainly makes me cry. Along those lines, most of the tables where I work have both date and time fields, but time is stored as seconds from midnight. In some cases the devs couldn't be bothered to set up a time field so when we look at inventory transactions I have a date that they occured, but no idea to know the order that they occured in on that date. You would think you could use base it off the transaction id, but the ids are assigned when the open transaction is created and since several people post transactions there's no guarantee they were posted in the correct order (or for that matter it was even posted on the day it was created).
|
# ? May 14, 2008 15:31 |
|
TSDK posted:If you ask me, then the original OpenSSL devs are just as much to blame for that clusterfuck as the guy who put in an incorrect 'fix' for the problem. It seems that in the comments to that links.org post that the OpenSSL devs are trying to cast all the blame on the Debian guy. Comments are things like "Oh he shouldn't have contacted openssl-dev (like most open source projects and the OpenSSL support page/README with the source says to), he should have contacted this obscure nowhere published address". And "The Debian guy's proposed patch was presented without context so that is why we said it was ok, but patches like this are so common that we have an FAQ (which may or may not have existed 2 years ago) to explicitly say patches of this kind are a bad idea, so it is really the Debian guy's fault for not scouring the mailing list archive looking for similar patches". The more I read coming from the OpenSSL team the more blame I think they have. It's too bad all the coverage I've seen of this (/. etc) don't mention OpenSSL's culpability at all.
|
# ? May 14, 2008 16:11 |
|
TSDK posted:It seems to me then that there were 3 WTFs: Did they rely on it, though? I thought the behavior was, if present, a source of bonus entropy. If absent, the other sources of entropy were still added to the mix so they weren't any worse off. The problem (from what someone else mentioned) is that the Debian "fix" managed to disable all sources of entropy. Anyway, I'm not sure if my understanding of this is correct as I've only really skimmed over the issue, but that's the impression that I got.
|
# ? May 14, 2008 19:33 |
|
Erasmus Darwin posted:Did they rely on it, though? I thought the behavior was, if present, a source of bonus entropy. If absent, the other sources of entropy were still added to the mix so they weren't any worse off. The problem (from what someone else mentioned) is that the Debian "fix" managed to disable all sources of entropy. Well they obviously are quite a bit worse off since taking the uninitialized data out as a source of entropy apparently led to weak keys.
|
# ? May 14, 2008 20:58 |
|
TheSleeper posted:Well they obviously are quite a bit worse off since taking the uninitialized data out as a source of entropy apparently led to weak keys.
|
# ? May 14, 2008 21:03 |
|
Regarding the OpenSSL vulnerability, http://metasploit.com/users/hdm/tools/debian-openssl/ It looks pretty bad-- quote:Removing this code has the side effect of crippling the seeding process for the OpenSSL PRNG. Instead of mixing in random data for the initial seed, the only "random" value that was used was the current process ID. On the Linux platform, the default maximum process ID is 32,768, resulting in a very small number of seed values being used for all PRNG operations. So for a given architecture and release, there's only 215 different keys. He completely brute-forced the two most commonly used keyspaces in about 62 CPU hours.
|
# ? May 15, 2008 04:24 |
|
I'll admit I've been doing the if (... == true) Now, before everyone jumps me I know this unnecessary. I've started doing it as I've worked with people who, when they review my code, find it much easier to follow if everything is explicitly defined. Before I started doing that I'd get asked a lot of questions explaining boolean conditions, since I've started explicitly stating them the questions have pretty much all stopped. I'm not sure why, but if it makes my life easier I guess I'll do it. This is the same reason why I've taken to the habit of always prefacing a class-wide variable/functions with 'this'. I've had many time where reviewers will ask where a variable/function comes from and since I've started doing this those questions don't come up anymore. The only thing I can think is that this improves readability for very very lazy reviewers.
|
# ? May 15, 2008 16:20 |
|
It's best to have boolean variable names that sound boolean, like hidden (or even isHidden). I prefix nonpublic class fields with _ -- it's simple and to the point. With respect to calling a method inside a class, I'm a bit conflicted: if there's no this., the method invoked has to be one belonging to the enclosing type, at least in C#.
|
# ? May 15, 2008 16:47 |
|
I understand what you mean, but that's not what's happening here. I'd get questioned for writingcode:
code:
The only thing I can think is that then you can glance and easly see if your checking for the true case or the false case, but even that is so simple the == shouldn't be necessary.
|
# ? May 15, 2008 23:03 |
|
CT2049 posted:The only thing I can think is that then you can glance and easly see if your checking for the true case or the false case, but even that is so simple the == shouldn't be necessary.
|
# ? May 15, 2008 23:09 |
|
Take a look at the source on this abomination of a website: http://forum.free-games.com.au/cgi-bin/gforum.cgi As a web designer with borderline OCD, the code of that made me so sick that I ended up rewriting with perfect XHTML 1.0 Strict and CSS compliance just so I could look at it and feel at peace. The first thing I noticed was the lack of proper headers, followed by the horrendous whitespacing. After that, it sort of blurred together in one ball of poo poo. I can only assume how terrible that forum software's codebase looks. DICTATOR OF FUNK fucked around with this message at 23:22 on May 15, 2008 |
# ? May 15, 2008 23:19 |
|
CT2049 posted:I understand what you mean, but that's not what's happening here. I'd get questioned for writing That's ridiculous. The first one reads like an english sentence; who could possible have a hard time reading it? It's like the difference between saying "my name is Bob" and "my name is Bob is a true statement".
|
# ? May 15, 2008 23:59 |
|
Today we made the switch from: Manually incrementing version numbers in the file, and putting them in the commit log, and manually cleaning up the mess when the version numbers caused svn conflicts. To: Committing, and letting svn handle the version stamp in the file. I did a lap of victory around the office when it happened, I've been begging to do this since day one, over three months ago. tef fucked around with this message at 00:31 on May 16, 2008 |
# ? May 16, 2008 00:25 |
|
I think this looks best:code:
|
# ? May 16, 2008 00:31 |
|
Victor posted:I think this looks best: I think there is something to be said for code:
|
# ? May 16, 2008 02:34 |
|
Luckily after today I'm no longer working with the guys who needed me to do that. Hooray! Although I'd like to try Victor's code:
|
# ? May 16, 2008 03:20 |
|
rather than have an equals method why not just overload operator== ? i see this sort of avoidance by old 'C' engineers all the time
|
# ? May 16, 2008 05:12 |
|
bcrules82 posted:rather than have an equals method why not just overload operator== ?
|
# ? May 16, 2008 05:35 |
|
bcrules82 posted:rather than have an equals method why not just overload operator== ? == is generally defined as reference equality, not data equality. Overriding it to perform data comparison would be a Coding Horror. Not to mention the need to give extra arguments to Equals() - case-sensitivity and comparison culture, in case of strings.
|
# ? May 16, 2008 05:52 |
|
There really should be an &= operator or something for referential equality.
|
# ? May 16, 2008 06:18 |
|
TRex EaterofCars posted:There really should be an &= operator or something for referential equality. Well don't these operators map into the extra math symbols, we have =, ≡, and ≣, so you could have === (≡) as data equality, and ==== (≣) for class equality. I don't have a degree in maths, when would you use "identical to" === (≡) and "strictly equivalent to" ==== (≣)? I just found this for possible usage in OOP: http://weblog.raganwald.com/2008/04/is-strictly-equivalent-to.html MrMoo fucked around with this message at 06:33 on May 16, 2008 |
# ? May 16, 2008 06:27 |
|
MrMoo posted:Well don't these operators map into the extra math symbols, we have =, ≡, and ≣, so you could have === (≡) as data equality, and ==== (≣) for class equality. You're right, it would be &== and not &=. As for the rest of that, I can't wait for Unicode keyboards. Using proper symbols would be a godsend, not some ==== bullshit. I know you can do Unicode in C# but typing in the codes is a pain in the dick.
|
# ? May 16, 2008 07:01 |
|
EssOEss posted:== is generally defined as reference equality, not data equality. Overriding it to perform data comparison would be a Coding Horror. Begone, foul Java demon!
|
# ? May 16, 2008 09:08 |
|
TSDK posted:No it wouldn't.
|
# ? May 16, 2008 10:44 |
|
Clearly the correct naming scheme is equal?, eq?, and eqv?
|
# ? May 16, 2008 15:33 |
|
EssOEss posted:== is generally defined as reference equality, not data equality. Overriding it to perform data comparison would be a Coding Horror. Not necessarily, in python for example `is` is used for reference equality.
|
# ? May 16, 2008 19:17 |
|
JoeNotCharles posted:Clearly the correct naming scheme is equal?, eq?, and eqv? code:
|
# ? May 16, 2008 19:35 |
|
TRex EaterofCars posted:As for the rest of that, I can't wait for Unicode keyboards. Using proper symbols would be a godsend, not some ==== bullshit. I know you can do Unicode in C# but typing in the codes is a pain in the dick. What the hell is a Unicode keyboard? I am imagining a keyboard with over 100,000 buttons here.
|
# ? May 17, 2008 00:40 |
|
|
# ? May 16, 2024 18:15 |
|
Melonhead posted:What the hell is a Unicode keyboard? I am imagining a keyboard with over 100,000 buttons here. A keyboard with a compose key? Maybe four or five different compose keys? Anything but ASCII in source code is infuriating as hell though.
|
# ? May 17, 2008 00:56 |