Welcome! Please see the About page for a little more info on how this works.

0 votes
in Clojure by

If you pass a non-positive value of 'n' or 'step' to partition, you get an infinite loop. Here are a few examples:

(partition 0 (link: 1 2 3))
(partition 1 -1 (link: 1 2 3))

Cause: partition and partition-all do not check that n and step are positive.

Approach: Add checks to partition and partition-all that n and step are positive.

Patch: clj-1647_3.patch

Prescreened by: Alex Miller

29 Answers

0 votes
by

Comment made by: alexmiller

Go for it

0 votes
by

Comment made by: alexmiller

Go for it

0 votes
by

Comment made by: alexmiller

Go for it

0 votes
by

Comment made by: alexmiller

Go for it

0 votes
by

Comment made by: alexmiller

Sorry, browser fail :)

0 votes
by

Comment made by: kworam

Thanks for picking it up Matthew, I appreciate it!

0 votes
by

Comment made by: kworam

Thanks for picking it up Matthew, I appreciate it!

0 votes
by
_Comment made by: alex+import_

New patch: clj-1647.patch

Includes tests, fewer whitespace changes, manually thrown IAEs. I'll do some basic benchmarking soon, although I expect the overhead to be quite low as we're only checking the arguments once.

Kevin, the reason your patch was working for partition-all but not partition is that partition is defined early-ish in the bootstrapping process and the {:pre .. :post ..} maps aren't read by defn until it's enhanced later on.
0 votes
by

Comment made by: kworam

Thanks for solving that mystery Matthew!

0 votes
by

Comment made by: alexmiller

Patch looks basically good. Minor changes:

  • internal-partition and internal-partition-all should be marked private with {{defn-}}.
  • Commit description should start with "CLJ-1647"
0 votes
by

Comment made by: mjg123

I added clj-1647_2.patch to supersede other patches on this issue. Jira ref is added to commit msg and defn- used where possible (defn- is not defined until after one of the private fns but there is the ^:private metadata added manually)

0 votes
by

Comment made by: alexmiller

The patch changes add-annotation from defn- to defn but that seems unrelated to the intent of the patch?

0 votes
by

Comment made by: mjg123

Thanks for looking so quickly Alex - sorry about that error in add-annotation. See clj-1647_3.patch

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJ-1647 (reported by kworam)
...