Bug #12202: nkf.c static void mime_putc the variable being used for loop used for outer loop too

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.1: WONTFIX, 2.2: DONE, 2.3: DONE

[fanantoxa (Anton Sivakov)]

for (i=0;i<mimeout_state.count;i++) {
  if (SP<mimeout_state.buf[i] && mimeout_state.buf[i]<DEL) {
    eof_mime();
    for (i=0;i<mimeout_state.count;i++) {
      (*o_mputc)(mimeout_state.buf[i]);
      base64_count++;
    }
    mimeout_state.count = 0;
  }
}

Variable 'i' here used for outer and inner loop.
Also variable mimeout_state.count can be changed within a loop it can affect outer loop

[shyouhei (Shyouhei Urabe)]

This is (surprisingly) safe, because that mimeout_state.count = 0; line purposefully terminates outer loop. It works as expected; just in a ultimately complicated way.

[fanantoxa (Anton Sivakov)]

Shyouhei Urabe wrote:

This is (surprisingly) safe, because that mimeout_state.count = 0; line purposefully terminates outer loop. It works as expected; just in a ultimately complicated way.

Is this ok? It can be complicated for understanding. Should I close this bug or try refactor?

[shyouhei (Shyouhei Urabe)]

I sent a pull request to its upstream (yes, nkf has its own project). So it's really up to its mainteners. I don't think this needs a hotfix in our copy.

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

Applied in changeset r54423.

---

• ext/nkf/nkf-utf8/nkf.c: Merge upstream 4f3edf80a0.
  patched by Anton Sivakov [Bug #12201] [Bug #12202]

[fanantoxa (Anton Sivakov)]

Yui NARUSE wrote:

Applied in changeset r54423.

---

• ext/nkf/nkf-utf8/nkf.c: Merge upstream 4f3edf80a0.
  patched by Anton Sivakov [Bug #12201] [Bug #12202]

Hi. I saw your changes and now it's:
...
for (j=0;i<mimeout_state.count;j++) {
(*o_mputc)(mimeout_state.buf[j]);
...

Now you're using 'j' but on condition you still using 'i'. Is it ok or mistake?

[naruse (Yui NARUSE)]

Anton Sivakov wrote:

Yui NARUSE wrote:

Applied in changeset r54423.

---

• ext/nkf/nkf-utf8/nkf.c: Merge upstream 4f3edf80a0.
  patched by Anton Sivakov [Bug #12201] [Bug #12202]

Hi. I saw your changes and now it's:
...
for (j=0;i<mimeout_state.count;j++) {
(*o_mputc)(mimeout_state.buf[j]);
...

Now you're using 'j' but on condition you still using 'i'. Is it ok or mistake?

fixed, thanks!

[usa (Usaku NAKAMURA)]

• Related to Bug #12201: nkf.c static void fold_conv part of exression always true added

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: WONTFIX, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: WONTFIX, 2.2: DONE, 2.3: REQUIRED

ruby_2_2 r54695 merged revision(s) 54423,54489.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.1: WONTFIX, 2.2: DONE, 2.3: REQUIRED to 2.1: WONTFIX, 2.2: DONE, 2.3: DONE

ruby_2_3 r54714 merged revision(s) 54423,54489.