Merge pull request #8 from Kinneyzhang/copilot/analyze-repo-issues

refactor: eliminate code duplication and fix correctness bugs across Elisp and C modules

_codeql_detected_source_root

@@ -0,0 +1 @@
+./ekp_c

ekp-utils.el

@@ -228,12 +228,6 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
         (setq boxes (ekp--flush-trailing-spaces spaces boxes))
         (vconcat (nreverse boxes))))))
 
-(defun ekp-clear-caches ()
-  (interactive)
-  (setq ekp-caches
-        (make-hash-table
-         :test 'equal :size 100 :rehash-size 1.5 :weakness nil)))
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (defun ekp-start-process-with-callback
@@ -257,12 +251,17 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
                    ,buffer-name))))
     process))
 
-(defun ekp-rust-module-reload (module)
+(defun ekp--module-reload (module)
+  "Load MODULE from a temp copy to allow rebuilding."
   (let ((tmpfile (make-temp-file
                   (file-name-nondirectory module))))
     (copy-file module tmpfile t)
     (module-load tmpfile)))
 
+;;; Rust Module Support (currently unused — ekp_rust/ directory does not exist)
+
+(defalias 'ekp-rust-module-reload #'ekp--module-reload)
+
 (defun ekp-module-dir ()
   (when-let ((root-dir (ekp-root-dir)))
     (expand-file-name "ekp_rust" root-dir)))
@@ -279,7 +278,7 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
   (if (executable-find "cargo")
       (let ((file (ekp-module-file)))
         (if file
-            (ekp-rust-module-reload file)
+            (ekp--module-reload file)
           (ekp-module-build)))
     (error "Please install cargo and add it to executable path!")))
 
@@ -293,10 +292,10 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
         ((eq system-type 'windows-nt)
          `("cmd.exe" "/c" ,(format "cd %s && cargo build -r"
                                    (ekp-module-dir))))
-        (t `("zsh" "-c" ,(format "cd %s && cargo build -r"
+        (t `(,shell-file-name "-c" ,(format "cd %s && cargo build -r"
                                  (ekp-module-dir)))))
        (lambda (proc buffer)
-         (ekp-rust-module-reload (ekp-module-file))
+         (ekp--module-reload (ekp-module-file))
          (message "ekp rust module reload success!")))
     (error "Please install cargo and add it to executable path!")))
 
@@ -322,12 +321,8 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
                               (t "ekp.so"))))
     (expand-file-name filename module-dir)))
 
-(defun ekp-c-module-reload (module)
+(defalias 'ekp-c-module-reload #'ekp--module-reload
-  "Load MODULE from a temp copy to allow rebuilding."
+  "Load MODULE from a temp copy to allow rebuilding.")
-  (let ((tmpfile (make-temp-file
-                  (file-name-nondirectory module))))
-    (copy-file module tmpfile t)
-    (module-load tmpfile)))
 
 (defun ekp-c-module-load ()
   "Load EKP C module if available."
@@ -368,7 +363,7 @@ Whitespace runs are preserved as separate boxes; CJK punctuation attaches to pre
          (cond
           ((eq system-type 'windows-nt)
            `("cmd.exe" "/c" ,(format "cd %s && make" module-dir)))
-          (t `("zsh" "-c" ,(format "cd %s && make" module-dir))))
+          (t `(,shell-file-name "-c" ,(format "cd %s && make" module-dir))))
          (lambda (proc buffer)
            (ekp-c-module-load)
            (message "ekp C module build success!")))

ekp.el

@@ -124,11 +124,7 @@ Used for explicit line breaks in poetry, code blocks, etc.")
   "Internal flag for parameter initialization.")
 
 ;;;; Initialization
-
+;; ekp-root-dir is provided by ekp-utils.el
-(defun ekp-root-dir ()
-  "Return directory containing ekp.el."
-  (when ekp--load-file
-    (file-name-directory ekp--load-file)))
 
 (defun ekp--load-dicts ()
   "Load hyphenation dictionaries."
@@ -530,35 +526,27 @@ Returns (:badness NUM :fitness NUM :gaps LIST :adjustment NUM :flexibility NUM).
           :adjustment adjustment
           :flexibility flexibility)))
 
-(defun ekp--hyphenate-p (hyphen-positions n)
+(defun ekp--sorted-vector-member-p (vec n)
-  "Return non-nil if position N ends with hyphenation.
+  "Return non-nil if N exists in sorted vector VEC.
-HYPHEN-POSITIONS is a sorted vector of indices where hyphenation can occur.
-Uses binary search for O(log n) lookup instead of O(n) linear search."
-  (and hyphen-positions
-       (> (length hyphen-positions) 0)
-       (let ((lo 0)
-             (hi (1- (length hyphen-positions))))
-         (while (< lo hi)
-           (let ((mid (/ (+ lo hi) 2)))
-             (if (< (aref hyphen-positions mid) n)
-                 (setq lo (1+ mid))
-               (setq hi mid))))
-         (= (aref hyphen-positions lo) n))))
-
-(defun ekp--flagged-p (flagged-positions n)
-  "Return non-nil if position N is a flagged (forced) break.
-FLAGGED-POSITIONS is a sorted vector of indices where forced breaks occur.
 Uses binary search for O(log n) lookup."
-  (and flagged-positions
+  (and vec
-       (> (length flagged-positions) 0)
+       (> (length vec) 0)
        (let ((lo 0)
-             (hi (1- (length flagged-positions))))
+             (hi (1- (length vec))))
          (while (< lo hi)
            (let ((mid (/ (+ lo hi) 2)))
-             (if (< (aref flagged-positions mid) n)
+             (if (< (aref vec mid) n)
                  (setq lo (1+ mid))
                (setq hi mid))))
-         (= (aref flagged-positions lo) n))))
+         (= (aref vec lo) n))))
+
+(defalias 'ekp--hyphenate-p #'ekp--sorted-vector-member-p
+  "Return non-nil if position N in HYPHEN-POSITIONS ends with hyphenation.
+HYPHEN-POSITIONS is a sorted vector of indices where hyphenation can occur.")
+
+(defalias 'ekp--flagged-p #'ekp--sorted-vector-member-p
+  "Return non-nil if position N in FLAGGED-POSITIONS is a flagged (forced) break.
+FLAGGED-POSITIONS is a sorted vector of indices where forced breaks occur.")
 
 ;;;; Dynamic Programming Line Breaking
 

ekp_c/ekp_module.h

@@ -155,6 +155,7 @@ typedef struct {
  */
 typedef struct {
     pthread_t threads[EKP_THREAD_POOL_SIZE];
+    size_t thread_count;
     pthread_mutex_t queue_lock;
     pthread_cond_t queue_cond;
     pthread_cond_t done_cond;

ekp_c/ekp_paragraph.c

@@ -311,6 +311,11 @@ ekp_paragraph_t *ekp_para_create(const char *text, size_t len,
         return NULL;
     }
 
+    if (!ekp_global) {
+        ekp_para_destroy(p);
+        return NULL;
+    }
+
     ekp_spacing_t *sp = &ekp_global->spacing;
 
     for (size_t i = 0; i < final_count; i++) {

ekp_c/ekp_thread_pool.c

@@ -92,6 +92,7 @@ ekp_thread_pool_t *ekp_pool_create(size_t num_threads)
         }
     }
 
+    pool->thread_count = num_threads;
     return pool;
 }
 
@@ -105,8 +106,7 @@ void ekp_pool_destroy(ekp_thread_pool_t *pool)
     pthread_cond_broadcast(&pool->queue_cond);
     pthread_mutex_unlock(&pool->queue_lock);
 
-    for (size_t i = 0; i < EKP_THREAD_POOL_SIZE; i++) {
+    for (size_t i = 0; i < pool->thread_count; i++) {
-        if (pool->threads[i])
         pthread_join(pool->threads[i], NULL);
     }