Ticket #886 (closed patch: fixed)

Opened 5 years ago

Last modified 5 years ago

[patch]various consting, memory leak fixes and so on.

Reported by: jimmy Owned by: jkeenan
Priority: normal Milestone:
Component: core Version: 1.4.0
Severity: medium Keywords:
Cc: fperrad, particle Language:
Patch status: Platform:

Description (last modified by jkeenan) (diff)

1. fixed some places that should be used with Parrot_str_free_cstring.
2. various consting.
3. other fixes.

Attachments

cage.patch Download (11.3 KB) - added by jimmy 5 years ago.
cage.2.patch Download (11.3 KB) - added by jimmy 5 years ago.
cage.3.patch Download (10.5 KB) - added by jimmy 5 years ago.
new patch
tt_886.patch Download (5.8 KB) - added by jimmy 5 years ago.
tt_886_win.patch Download (2.7 KB) - added by jimmy 5 years ago.
edited.tt_886_win.patch Download (1.7 KB) - added by jkeenan 5 years ago.
Jimmy's tt_win32.patch, edited to eliminate one file duplicated in another tt_886.patch

Change History

Changed 5 years ago by jimmy

Changed 5 years ago by jimmy

  Changed 5 years ago by jimmy

please use the cage.2.patch, the cage.patch is not right.

  Changed 5 years ago by dukeleto

Howdy,

I applied this patch to r41123, only one hunk failed, but I am getting a failing test:

$ prove -v t/op/spawnw.t

ok 1 - exit code: 0
ok 2 - exit code: 123
ok 3 - exit code: 3
ok 4 - exit code: 0
ok 5 - exit code: 123
ok 6 - exit code: 3
not ok 7 - grow argv buffer

#   Failed test 'grow argv buffer'
#   at t/op/spawnw.t line 141.
#          got: 'return code: 255
# '
#     expected: 'return code: 10
# '
# Looks like you failed 1 test of 7.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/7 subtests 

Test Summary Report
-------------------
t/op/spawnw.t (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1

If you can attach some smaller patches, that fix each file, it will be easier to figure out what is causing this test failure. I did apply one part of the patch that fixes some documentation in src/pmc/env.pmc .

  Changed 5 years ago by dukeleto

  • patch changed from new to rejected

Changed 5 years ago by jimmy

new patch

  Changed 5 years ago by jimmy

Hello, I added the newest patch, please try again.

follow-up: ↓ 6   Changed 5 years ago by jkeenan

  • component changed from none to core

in reply to: ↑ 5 ; follow-up: ↓ 7   Changed 5 years ago by jkeenan

  • owner set to jkeenan
  • status changed from new to assigned
  • patch rejected deleted

Replying to jkeenan:

jimmy and I discussed this on IRC. Some files touched by this patch have changed since the patches were originally created. So I'm recommending jimmy touch these up and submit in two separate patches -- one for the 3 Win32-specific files and one for the other 7.

kid51

Changed 5 years ago by jimmy

Changed 5 years ago by jimmy

in reply to: ↑ 6 ; follow-up: ↓ 8   Changed 5 years ago by jkeenan

  • cc fperrad, particle added
  • description modified (diff)

Replying to jkeenan:

Replying to jkeenan: jimmy and I discussed this on IRC. Some files touched by this patch have changed since the patches were originally created. So I'm recommending jimmy touch these up and submit in two separate patches -- one for the 3 Win32-specific files and one for the other 7.

1. Should src/pmc/string.pmc be in both patches?
2. I'll test the non-Win32 patch again after tomorrow's release.
3. I'm going to cc some our Win32 developers to ask them to test that patch after tomorrow's release.

Thank you very much.
kid51

in reply to: ↑ 7   Changed 5 years ago by jimmy

Replying to jkeenan:

Replying to jkeenan:

Replying to jkeenan: jimmy and I discussed this on IRC. Some files touched by this patch have changed since the patches were originally created. So I'm recommending jimmy touch these up and submit in two separate patches -- one for the 3 Win32-specific files and one for the other 7.

1. Should src/pmc/string.pmc be in both patches?
2. I'll test the non-Win32 patch again after tomorrow's release.
3. I'm going to cc some our Win32 developers to ask them to test that patch after tomorrow's release. Thank you very much.
kid51

1. I don't think. It just do the same check like other pmc's mark() function.

Changed 5 years ago by jkeenan

Jimmy's tt_win32.patch, edited to eliminate one file duplicated in another tt_886.patch

  Changed 5 years ago by cotto

tt_886.patch was applied with a minor change as r43080. Thanks.

follow-up: ↓ 11   Changed 5 years ago by fperrad

in reply to: ↑ 10   Changed 5 years ago by jkeenan

Replying to fperrad:

tt_886_win.patch is OK with mingw.

I applied the edited version of this patch to trunk in r43188.

Since I don't have Win32 myself, I'd appreciate any reports of test failures.

If I don't get any in 7 days, I will close this ticket.

Thank you very much.
kid51

follow-up: ↓ 13   Changed 5 years ago by jkeenan

JimmyZ reported on IRC that this passed on Win32. Will keep open a few more days for other reports.

Thank you very much.
kid51

in reply to: ↑ 12 ; follow-up: ↓ 14   Changed 5 years ago by jkeenan

Replying to jkeenan:

JimmyZ reported on IRC that this passed on Win32. Will keep open a few more days for other reports.

I'm not seeing any Smolder test failures that can be attributed to the patches applied to this ticket. However, we are getting failures on our Win32 smoke tests in t/library/pcre.t.

1..2
ok 1 - libpcre loading
not ok 2 - soup to nuts

#   Failed test 'soup to nuts'
#   at t/library/pcre.t line 104.
#          got: 'ok 1
# ok 2
# not ok 3
# not ok 4
# not ok 5
# '
#     expected: 'ok 1
# ok 2
# ok 3
# ok 4
# ok 5
# '
# Looks like you failed 1 test of 2.

I can't attribute those failures to the Win32 patch which was the last part applied from this ticket, but, out of caution, I am going to hold the ticket open.

kid51

in reply to: ↑ 13 ; follow-up: ↓ 15   Changed 5 years ago by jkeenan

Replying to jkeenan:

> I'm not seeing any Smolder test failures that can be attributed 
> to the patches applied to this ticket. However, we are getting 
> failures on our Win32 smoke tests in ''t/library/pcre.t''.

I have opened TT #1401 to track that issue. So this ticket becomes dependent on TT #1401 for resolution.

in reply to: ↑ 14   Changed 5 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to jkeenan:

In the last three days Ron Blasch++ has submitted lots of Smolder reports on various Win32 platforms. While he is getting some test failures in most of these submissions, he is not getting failures in t/library/pcre.t. So I'm going to go out on a limb and say that the failures we are getting from other smoke testers are due at least in part to conditions on their respective boxes.

I am therefore going to declare this ticket's status independent of the status of TT #1401 and am resolving this ticket.

Thanks again to jimmy and rblasch.

kid51

Note: See TracTickets for help on using tickets.