Skip to content

Commit 049b3c7

Browse files
fglockfglock
committed
fix pack() WIP - refactoring
1 parent 00c6da3 commit 049b3c7

File tree

2 files changed

+117
-39
lines changed

2 files changed

+117
-39
lines changed

‎src/main/java/org/perlonjava/operators/Pack.java‎

Lines changed: 58 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -184,33 +184,11 @@ public static RuntimeScalar pack(RuntimeList args) {
184184
}
185185

186186
// Check if this is a numeric format followed by '/' - skip it entirely
187-
if ((isNumericFormat(format) || format == 'Z') && i + 1 < template.length()) {
188-
// Look ahead, skipping modifiers and counts
189-
int lookAhead = i + 1;
190-
System.err.println("DEBUG: checking if numeric format '" + format + "' is followed by '/'");
191-
192-
// Skip modifiers
193-
while (lookAhead < template.length() &&
194-
(template.charAt(lookAhead) == '<' ||
195-
template.charAt(lookAhead) == '>' ||
196-
template.charAt(lookAhead) == '!')) {
197-
lookAhead++;
198-
}
199-
200-
// Skip count or *
201-
if (lookAhead < template.length() && template.charAt(lookAhead) == '*') {
202-
lookAhead++;
203-
} else if (lookAhead < template.length() && Character.isDigit(template.charAt(lookAhead))) {
204-
while (lookAhead < template.length() && Character.isDigit(template.charAt(lookAhead))) {
205-
lookAhead++;
206-
}
207-
}
208-
209-
// Check if followed by '/'
210-
if (lookAhead < template.length() && template.charAt(lookAhead) == '/') {
211-
// Skip this entire format sequence - it's used for length encoding
212-
System.err.println("DEBUG: skipping format sequence ending at position " + (lookAhead - 1));
213-
i = lookAhead - 1; // -1 because loop will increment
187+
if (isNumericFormat(format) || format == 'Z') {
188+
int slashPos = PackHelper.checkForSlashConstruct(template, i);
189+
if (slashPos != -1) {
190+
System.err.println("DEBUG: skipping format '" + format + "' at position " + i + " because it's part of '/' construct");
191+
i = slashPos - 1; // -1 because loop will increment
214192
continue;
215193
}
216194
}
@@ -307,9 +285,34 @@ public static RuntimeScalar pack(RuntimeList args) {
307285
for (int j = 0; j < count; j++) {
308286
output.write(0);
309287
}
310-
} else if (format == '/') {
311-
// System.err.println("DEBUG: entering '/' handler, valueIndex=" + valueIndex);
312-
// '/' must follow a numeric type or Z
288+
} else if (format == '/') {
289+
System.err.println("DEBUG: entering '/' handler, i=" + i);
290+
291+
// FIRST check if '/' is at the start of template (invalid)
292+
if (i == 0) {
293+
throw new PerlCompilerException("Invalid type '/'");
294+
}
295+
296+
// THEN check what follows '/'
297+
// Always use i+1 because i points to '/' itself
298+
if (i + 1 >= template.length()) {
299+
throw new PerlCompilerException("Code missing after '/'");
300+
}
301+
System.err.println("DEBUG: checking character at position " + i);
302+
char afterSlash = template.charAt(i + 1); // Always i+1!
303+
System.err.println("DEBUG: character after '/' is '" + afterSlash + "' (code " + (int)afterSlash + ")");
304+
305+
// Check if '/' is followed by a repeat count (which is an error)
306+
if (afterSlash == '*' || Character.isDigit(afterSlash)) {
307+
throw new PerlCompilerException("'/' does not take a repeat count");
308+
}
309+
310+
// Check if it's a valid string format
311+
if (afterSlash != 'a' && afterSlash != 'A' && afterSlash != 'Z') {
312+
throw new PerlCompilerException("'/' must be followed by a string type");
313+
}
314+
315+
// NOW check if '/' follows a numeric type
313316
if (i == 0) {
314317
throw new PerlCompilerException("Invalid type '/'");
315318
}
@@ -372,15 +375,8 @@ public static RuntimeScalar pack(RuntimeList args) {
372375
}
373376

374377
// Get the string format that follows '/'
375-
if (i + 1 >= template.length()) {
376-
throw new PerlCompilerException("'/' must be followed by a string type");
377-
}
378-
379378
i++; // move to string format
380379
char stringFormat = template.charAt(i);
381-
if (stringFormat != 'a' && stringFormat != 'A' && stringFormat != 'Z') {
382-
throw new PerlCompilerException("'/' must be followed by a string type");
383-
}
384380

385381
// Parse count for string format
386382
int stringCount = -1; // -1 means use full string
@@ -480,8 +476,31 @@ public static RuntimeScalar pack(RuntimeList args) {
480476
if (stringFormat == 'Z' && stringCount < 0) {
481477
output.write(0); // null terminator
482478
}
483-
} else if (format == '/') {
484-
// ... existing '/' handler code ...
479+
} else if (format == '@') {
480+
// @ is used for absolute positioning
481+
// '/' uses the previously packed numeric value as a count for the following string format
482+
483+
// First, check what follows '/'
484+
if (i + 1 >= template.length()) {
485+
throw new PerlCompilerException("Code missing after '/'");
486+
}
487+
488+
i++; // move to next character after '/'
489+
char nextChar = template.charAt(i);
490+
491+
// Check if '/' is followed by a repeat count (which is an error)
492+
// This includes '*' or any digit
493+
if (nextChar == '*' || Character.isDigit(nextChar)) {
494+
throw new PerlCompilerException("'/' does not take a repeat count");
495+
}
496+
497+
// Check if it's a valid string format
498+
if (nextChar != 'a' && nextChar != 'A' && nextChar != 'Z') {
499+
throw new PerlCompilerException("'/' must be followed by a string type");
500+
}
501+
502+
// Now we can continue processing...
503+
// (rest of the handler code)
485504
} else if (format == '@') {
486505
// @ is used for absolute positioning
487506
// @n means null-fill or truncate to position n

‎src/main/java/org/perlonjava/operators/PackHelper.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,27 @@ static void writeBER(ByteArrayOutputStream output, long value) {
155155
}
156156
}
157157

158+
/**
159+
* Validates the '/' format in pack/unpack templates.
160+
*/
161+
public static char validateSlashFormat(String template, int position) {
162+
if (position + 1 >= template.length()) {
163+
throw new PerlCompilerException("Code missing after '/'");
164+
}
165+
166+
char afterSlash = template.charAt(position + 1);
167+
168+
if (afterSlash == '*' || Character.isDigit(afterSlash)) {
169+
throw new PerlCompilerException("'/' does not take a repeat count");
170+
}
171+
172+
if (afterSlash != 'a' && afterSlash != 'A' && afterSlash != 'Z') {
173+
throw new PerlCompilerException("'/' must be followed by a string type");
174+
}
175+
176+
return afterSlash;
177+
}
178+
158179
/**
159180
* Writes a short integer to the output stream in little-endian order.
160181
*
@@ -347,4 +368,42 @@ static void writeBitString(ByteArrayOutputStream output, String str, int count,
347368
output.write(byteValue);
348369
}
349370
}
371+
372+
/**
373+
* Check if a format at the given position is part of a '/' construct.
374+
* For example, in "n/a*", the 'n' is part of a '/' construct.
375+
*
376+
* @param template The template string
377+
* @param position The position of the format character
378+
* @return The position of '/' if found, or -1 if not part of '/' construct
379+
*/
380+
public static int checkForSlashConstruct(String template, int position) {
381+
int lookAhead = position + 1;
382+
383+
// Skip modifiers (<, >, !)
384+
while (lookAhead < template.length() &&
385+
(template.charAt(lookAhead) == '<' ||
386+
template.charAt(lookAhead) == '>' ||
387+
template.charAt(lookAhead) == '!')) {
388+
lookAhead++;
389+
}
390+
391+
// Skip count or *
392+
if (lookAhead < template.length()) {
393+
if (template.charAt(lookAhead) == '*') {
394+
lookAhead++;
395+
} else if (Character.isDigit(template.charAt(lookAhead))) {
396+
while (lookAhead < template.length() && Character.isDigit(template.charAt(lookAhead))) {
397+
lookAhead++;
398+
}
399+
}
400+
}
401+
402+
// Check if followed by '/'
403+
if (lookAhead < template.length() && template.charAt(lookAhead) == '/') {
404+
return lookAhead;
405+
}
406+
407+
return -1;
408+
}
350409
}

0 commit comments

Comments
 (0)