Hacker News Reader

Comment by micw | Hacker News Reader

Comparing both patches, I see a clear difference. While the contribution from the linked article changes the behaviour of both branches of the if...else, the final contribution changes only the parth in the "if" branch.

I guess, the maintainer had some reason to change the code. Nevertheless, giving credits to the original patch author would be appropriate since the actual fix was almost 100% copied.

View on HN

Same for the original patch from the mailing conversation above. The original results in (reformatted)

  #ifdef CONFIG_PPC32
       *data = ((unsigned int *)child->thread.fp_state.fpr)[FPRINDEX(index)];
  #else
        flush_fp_to_thread(child);
        if (fpidx < (PT_FPSCR - PT_FPR0))
                memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long));
        else
                *data = child->thread.fp_state.fpscr;
  #endif

while the final patch is

        flush_fp_to_thread(child);
 if (fpidx < (PT_FPSCR - PT_FPR0)) {
  if (IS_ENABLED(CONFIG_PPC32))
   *data = ((u32 *)child->thread.fp_state.fpr)[fpidx];
  else
   memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long));
 } else
   *data = child->thread.fp_state.fpscr;

So there is actually a difference between both solutions. The original has neither the flush_fp_to_thread not the else-condition for the PPC32 architecture. I cannot say which is right/wrong/better but it's definitively a different result.

There's a clear logical difference in those two patches and I can't blame the maintainer for going with his version.

Attribution should be given though, regardless of the bug OPs patch introduced the actual line that fixes the issue is clearly the same thing with the appropriate coding style.

As a maintainer, I can understand his reasoning but TBH it would've been easier and more respectful to just reply with the proper code snippet, having the contributor submit it (if he agreed) and then giving them credit.

Honestly, the comment section of this thread sucks, the level of self righteousness, "aggression" and gatekeeping is ridiculously uncalled for.

Agreed. You basically get paid in reputation, trustworthiness, and maybe even beer/brownie/pizza points.

I also think that public interactions offer a preview of how easy (or difficult) it will be to work with someone on a project... we're not experts on every domain of knowledge, so the best bet is to approach interactions with a bit of humility and respect.

If this were an AITA reddit thread, ESH (a little bit).

As an old C programmer, I'd be wary of code sent to me with a bunch of custom macros. That might indicate a certain immaturity with C. Maintaining a large code base is part fire suppression and part avoiding name collisions.

I'm the author of the post. Unfortunately, I cannot link to the original patch I sent (it was very different) because I only sent it to the Linux kernel security mailing list. I wish I had saved it somewhere so I could show it.

If your mail client doesn't have a "sent" folder, you need a better mail client :)

(Totally with you though: maintainer was a douche. People are the worst.)

Should be in your "Sent" folder right?

Thanks, I actually have it in the sent folder of my personal email, I forgot about this. How can I share the email thread with you?

Ok I've forwarded the email thread: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/m...

Forwarding a random email conversation from a year+ ago without any context to public mailing list that is read who knows how many people just to get it into an archive so that you can link to it in HN comment does not really help your case.

And if you care so much about getting your code into the kernel you should consider replying to this: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/m...

On a related note Christophe Leroy should be applauded for his calm and matter of fact response. I don’t know if I would be capable of that.

You can export the eml of the sent mail

Reply on news.ycombinator.com