Not to throw shade, just wishing that somebody here can understand. Whenever an input is reasonably long, an analyzing function will crash, and this PR aims to fix that with a mechanism that contradicts the maintainer’s understanding while a similar C implementation does not need this fix. Clearly, the maintainer has not heard a certain programming mantra…

  • jwaters42@beehaw.org
    link
    fedilink
    arrow-up
    47
    ·
    7 months ago

    After the xz debacle, I think we should resist the temptation to rush maintainers into accepting code that they don’t fully understand.

      • towerful@programming.dev
        link
        fedilink
        arrow-up
        24
        ·
        7 months ago

        No you aren’t.
        The title is demeaning.
        “don’t want to throw shade” as if we all know what’s going on except the people involved in the link.
        “Clearly the maintainer hasn’t heard of a certain…” Haha, we are in the same group, right? We know what’s up, guys… Right? Haha, look at these losers.

        Never mind that you are applying a time pressure on open source maintainers to try and merge a change they don’t understand. Not very respectful. And quite frankly, in extremely bad taste considering the recently revealed xz social engineering.

        Where is the question?
        There isn’t a single question mark in your post. You frame it as if their problem and they don’t understand.
        Even here, where you are so close to asking, you make it sound like you are checking that everyone here understands.

  • gedhrel@lemmy.world
    link
    fedilink
    arrow-up
    45
    ·
    7 months ago

    Incidentally, this kind of passive-aggressive pressure is the kind of thing that might be considered a legitimate security threat, post xz. If you need to vent, vent in private. If “it works for you” but the maintainer is asking legitimate questions about the implementation, consider engaging with that in good faith and evaluating their questions with an open mind.

    • lysdexic@programming.dev
      link
      fedilink
      English
      arrow-up
      10
      ·
      7 months ago

      Incidentally, this kind of passive-aggressive pressure is the kind of thing that might be considered a legitimate security threat, post xz.

      Yes, OP’s attempt to bully a maintainer into accepting his PR is a very shitty thing to do.

      Throwing veiled personal attacks, such as insinuating a developer is incompetent or dumb, is also very bad form.

      This says more about OP than anything. I hope I never have to work with anyone like that. What a shit show of a person.

  • Solemarc@lemmy.world
    link
    fedilink
    arrow-up
    26
    ·
    7 months ago

    What mantra? I think this maintainer is doing the right thing here by trying to understand why this fix works.

    You should always attempt to address the root cause of an issue instead of slapping band aid patches onto everything.

    To me it looks like the maintainer is trying to find out what exactly is wrong. “this doesn’t happen in our C implementation” implies that there’s something wrong with the rust code specifically.

  • Ogeon@programming.dev
    link
    fedilink
    arrow-up
    16
    ·
    7 months ago

    A “mantra” more programmers should have is to fix the cause of the issue, and not just the symptoms. You have to understand what the problem is to be able to fix it.

  • Giooschi@lemmy.world
    link
    fedilink
    English
    arrow-up
    11
    ·
    7 months ago

    while a similar C implementation does not need this fix

    No, that implementation also needs the fix. It’s just that it was never properly tested, so they thought it was working correctly.

    • Aatube@kbin.melroy.orgOP
      link
      fedilink
      arrow-up
      1
      arrow-down
      3
      ·
      edit-2
      7 months ago

      They tested the same strings on that implementation., though judging by the recent comments someone’s found something.

      • Giooschi@lemmy.world
        link
        fedilink
        English
        arrow-up
        7
        ·
        7 months ago

        They tested the same strings on that implementation

        The strings were the same, but not the implementation. They were testing the decoding of the strings, but the C function they were looking at was the one for encoding them. The decoding function was correct but what it read didn’t match the encoding one.

        though judging by the recent comments someone’s found something.

        Yeah, that’s me :)

  • tyler@programming.dev
    link
    fedilink
    arrow-up
    4
    ·
    7 months ago

    Looks to me like a reasonable conversation is happening trying to find the underlying issue. Where is the PR being held up?

    • Aatube@kbin.melroy.orgOP
      link
      fedilink
      arrow-up
      1
      arrow-down
      3
      ·
      edit-2
      7 months ago

      Since I posted it, the conversation has moved forward. Before that it was just two people being justifiably confused.

      • Maddier1993@programming.dev
        link
        fedilink
        arrow-up
        4
        ·
        7 months ago

        You must understand that maintainers need to worry about supply chain attacks ever since the xz debacle. So I suggest you wait.

        • Aatube@kbin.melroy.orgOP
          link
          fedilink
          arrow-up
          1
          ·
          7 months ago

          Thanks. All I wanted was to have someone answer with what’s actually going on, and thanks to Giooschi below that has happened, and there indeed isn’t anything to do at this point. I’ve tried to edit the post but couldn’t find a proper wording.

  • Miaou
    link
    fedilink
    arrow-up
    2
    ·
    7 months ago

    OP is welcome to use a patched version in their repo and stop harassing maintainers