7 ms·

Ah such classic outsourced third-world contractor output. They are told what to do (by some PM who has no idea) and they do it. If they ever say "uh this doesn'

Ah such classic outsourced third-world contractor output. They are told what to do (by some PM who has no idea) and they do it. If they ever say "uh this doesn't make sense" they are cheaply replaced. I've seen some of this nonsense (though not IBM related): strncpy(dst, src, strlen(src)); dst[strlen(dst)] = '\0';

zeusk 8y ago
> dst[strlen(dst)] = '\0'; That line alone is cancerous.
- copperx 8y ago
  That line belongs in a hall of shame somewhere in the internet.
- ggg9990 8y ago
  Not a programmer. What’s so bad about it?
  - jonathankoren 8y ago
    It doesn’t make any sense. The way a string is terminated in C is by a null character. ‘\0’ is a mnemonic. strlen() returns the length of a strong. It does this by doing a linear scan down the array until it finds the null, and the returns that index as the length. That line of code does nothing useful. It means, “scan through the array until you find a null, then write a null there.” Which is exactly what you had to start. So why would someone write this? Well, they were told to make sure that you always terminate your C strings with a null, otherwise you’ll run off the end of the array and corrupt memory. This is true, but this isn’t how you need to find the end of the array. They should have used the number from strlen(src), not strlen(dest). Even worse, strncpy() will put the null terminator in dest in this case. The line belies any sense that the programmer had any understanding of what the code was actually doing. It’s a amateur mistake.
    - ggg9990 8y ago
      Thanks. Great explanation.
    - zeusk 8y ago
      using strlen(src) is also an amateur mistake, because the whole point of doing that (stncpy family, ensuring dst ends with null) is that dst might not be large enough to fit string from src. It has to be the size of dst, which can only be known at the site of allocation in C (except for a non-clean way of looking at heap metadata).
      - jonathankoren 8y ago
        There’s no reason to believe from these two lines that the memory is insufficiently allocated. It’s just two lines. The point wasn’t to share a safe string copy routine, the point of the post was to illustrate a single mistake.
        zeusk 8y ago
        I'm not assuming that, but then you can't assume the memory is sufficient either. If the string is from network or user, then you can never assume the buffer to be of sufficient size (which they definitely do, because they aren't tracking how much of src was copied into allocated dst).
        ploxiln 8y ago
        Each line, individually, is total nonsense. It never makes sense to use strlen() for an argument to strncpy(). Only the size of the destination buffer makes sense for strncpy() (usually sizeof(dst)). For other cases you would probably use memcpy().
    - projectramo 8y ago
      pace @zeusk: I thought the point was that they didn't know if the dest is long enough to copy source, so they copy as much of it as they can. The reason for the '\0' is in the case that src is shorter than dest. A more explicit way may be to strlen dest > src then just use lenght(src) otherwise use str(dest). But this might actually be more efficient. Edit: And in this case Spolsky doesn't apply because it seems the developer doesn't get to allocate the dest length.
  - nazri1 8y ago
    See also: Shlemiel the painter [1] [1] https://www.joelonsoftware.com/2001/12/11/back-to-basics/ https://www.joelonsoftware.com/2001/12/11/back-to-basics/
forapurpose 8y ago
> outsourced third-world contractor output Do we know that the developers were outsourced or which countries they were from?
- ballenf 8y ago
  Yes, in an inferred way at least. Look at the IBM jobs / careers page and locations. You can see where different types of openings are. Last year I looked at the AI jobs because I was curious where they were doing the work. Mumbai was first in terms of open positions for machine learning, if I recall correctly.
  - forapurpose 8y ago
    That tells us nothing about where the code discussed higher up this thread was written.
snambi 8y ago
Sounds like total failure of management
de_Selby 8y ago
> by some PM who has no idea This is the real problem. That PM of course wants the work done yesterday and has ego issues so even when the contractor does question it they just double down on what they originally said.

zeusk 8y ago

> dst[strlen(dst)] = '\0'; That line alone is cancerous.

copperx 8y ago
That line belongs in a hall of shame somewhere in the internet.
ggg9990 8y ago
Not a programmer. What’s so bad about it?
- jonathankoren 8y ago
  It doesn’t make any sense. The way a string is terminated in C is by a null character. ‘\0’ is a mnemonic. strlen() returns the length of a strong. It does this by doing a linear scan down the array until it finds the null, and the returns that index as the length. That line of code does nothing useful. It means, “scan through the array until you find a null, then write a null there.” Which is exactly what you had to start. So why would someone write this? Well, they were told to make sure that you always terminate your C strings with a null, otherwise you’ll run off the end of the array and corrupt memory. This is true, but this isn’t how you need to find the end of the array. They should have used the number from strlen(src), not strlen(dest). Even worse, strncpy() will put the null terminator in dest in this case. The line belies any sense that the programmer had any understanding of what the code was actually doing. It’s a amateur mistake.
  - ggg9990 8y ago
    Thanks. Great explanation.
  - zeusk 8y ago
    using strlen(src) is also an amateur mistake, because the whole point of doing that (stncpy family, ensuring dst ends with null) is that dst might not be large enough to fit string from src. It has to be the size of dst, which can only be known at the site of allocation in C (except for a non-clean way of looking at heap metadata).
    - jonathankoren 8y ago
      There’s no reason to believe from these two lines that the memory is insufficiently allocated. It’s just two lines. The point wasn’t to share a safe string copy routine, the point of the post was to illustrate a single mistake.
      - zeusk 8y ago
        I'm not assuming that, but then you can't assume the memory is sufficient either. If the string is from network or user, then you can never assume the buffer to be of sufficient size (which they definitely do, because they aren't tracking how much of src was copied into allocated dst).
      - ploxiln 8y ago
        Each line, individually, is total nonsense. It never makes sense to use strlen() for an argument to strncpy(). Only the size of the destination buffer makes sense for strncpy() (usually sizeof(dst)). For other cases you would probably use memcpy().
  - projectramo 8y ago
    pace @zeusk: I thought the point was that they didn't know if the dest is long enough to copy source, so they copy as much of it as they can. The reason for the '\0' is in the case that src is shorter than dest. A more explicit way may be to strlen dest > src then just use lenght(src) otherwise use str(dest). But this might actually be more efficient. Edit: And in this case Spolsky doesn't apply because it seems the developer doesn't get to allocate the dest length.
- nazri1 8y ago
  See also: Shlemiel the painter [1] [1] https://www.joelonsoftware.com/2001/12/11/back-to-basics/ https://www.joelonsoftware.com/2001/12/11/back-to-basics/

forapurpose 8y ago

> outsourced third-world contractor output Do we know that the developers were outsourced or which countries they were from?

ballenf 8y ago
Yes, in an inferred way at least. Look at the IBM jobs / careers page and locations. You can see where different types of openings are. Last year I looked at the AI jobs because I was curious where they were doing the work. Mumbai was first in terms of open positions for machine learning, if I recall correctly.
- forapurpose 8y ago
  That tells us nothing about where the code discussed higher up this thread was written.

snambi 8y ago

Sounds like total failure of management

de_Selby 8y ago

> by some PM who has no idea This is the real problem. That PM of course wants the work done yesterday and has ego issues so even when the contractor does question it they just double down on what they originally said.