Debug msgs - #299
Conversation
| { | ||
| fdpp->print(FDPP_PRINT_LOG, format, vl); | ||
| if (format[0] == '@') | ||
| fdpp->print(FDPP_PRINT_LOG_NOPREFIX, format + 1, vl); |
There was a problem hiding this comment.
Just add flag, shift it to
16 to not clash with the value.
f_node's f_count field was removed back in 2009 (bfdad2a), so remove its use from debug message and the corresponding text.
1/ Fix up the FAR string printing. Need to use standard format specifiers and convert args to be compatible. 2/ Fix up type width format specifiers.
Now that DebugPrintf are prefixed with "fdpp: " we need to prefix those that should be appended. Was: ===== fdpp: SUBSTing from: C:\ fdpp: NLS: DosUpFMem(): len=11, f997:03c0="fdpp: \fdpp: Tfdpp: Efdpp: Sfdpp: Tfdpp: Ifdpp: Tfdpp: .fdpp: Bfdpp: Afdpp: Tfdpp: " fdpp: NLS: nlsFUpMem() fdpp: NLS: upMMem(): len=11, f997:03c0="fdpp: \fdpp: Tfdpp: Efdpp: Sfdpp: Tfdpp: Ifdpp: Tfdpp: .fdpp: Bfdpp: Afdpp: Tfdpp: " fdpp: NLS: upMMem(): result="fdpp: \fdpp: Tfdpp: Efdpp: Sfdpp: Tfdpp: Ifdpp: Tfdpp: .fdpp: Bfdpp: Afdpp: Tfdpp: " Now: ===== fdpp: SUBSTing from: C:\ fdpp: NLS: DosUpFMem(): len=11, f997:03c0="\TESTIT.BAT" fdpp: NLS: nlsFUpMem() fdpp: NLS: upMMem(): len=11, f997:03c0="\TESTIT.BAT" fdpp: NLS: upMMem(): result="\TESTIT.BAT"
|
I have a crash in truename at adjust_far() once I enable the debugging. The trigger is a super long argument (128 chars) which actually was unnecessary so I have removed it in my dosemu branch. Not sure whether it's important, after all it's a ridiculous size, but I thought I'd mention it. |
| va_list vl; | ||
| int prio = FDPP_PRINT_LOG; | ||
|
|
||
| if (strcmp("%s", format) == 0) { // The string has been preformatted and we |
There was a problem hiding this comment.
I think it would be reasonable
to demand @ in a format string
itself. I'd like to avoid va_arg() here.
|
|
I already looked for a couple of hours, but my lack of C++ knowledge stops me from finding anything other than it occurs in the call
1/ Yes, use dosemu devel (still has the excessively long and unnecessary argument in the batch file Here's mine. |
to start with. Why is this a minimal reproducer?
|
|
I suppose you meant this link: |
|
|
You didn't set up the like the instructions said |
|
or more likely you did but didn't use the command it printed (what you posted looks like mine) |
|
The problem is that _vsnprintf() |
|
Are you set on musl it looked quite complicated to extract the salient part? I looked for an alternative, this popped up https://github.com/nothings/stb/blob/master/stb_sprintf.h license is MIT or Public Domain. |
|
I am not set for musl by |
|
No, it's Public Domain OR MIT, the user gets to choose which he wants to follow. |
|
That is written in the end, and |
|
Ok, choose MIT and remove |
|
Will the new vsnprintf need to be in the kernel C code or in the C++ part? |
|
Just replace the buggy prf.c. |
|
Can you tell me what implementation of put_console() is used for fdpp, it's all ordered and not obvious which was built in the old pdf.?. also do I need to implement hexd, there's only one use of it outside of per.c? |
|
As surprising as it may seem, I am pretty sure this one: MK_FAR_SCP() hexd() - just copy/paste it? |
|
I made a draft PR that seems to work #300, but I wonder if there's some way that _sprintf() could do the GET_PTR/GET_FP32 translation instead of them being splattered all over the source? |
|
Possibly, but not in this PR, since
currently they are already "splattered".
I dont think it is possible to do so
w/o either adding new formats
(unwanted) or do some nasty mods
in the generic printf code, but I didn't
think of that a lot. Try to find a
minimally invasive approach, this
will require some creativity.
Четверг, 9 июля 2026 г получено от Andrew Bird:
… andrewbird left a comment (dosemu2/fdpp#299)
I made a draft PR that seems to work #300, but I wonder if there's some way that _sprintf() could do the GET_PTR/GET_FP32 translation instead of them being splattered all over the source?
--
Reply to this email directly or view it on GitHub:
#299 (comment)
You are receiving this because you commented.
Message ID: ***@***.***
|
|
I think actually this can be done by
adding just 1 new conversion for all
fdpp types, eg %A (auto) that will
extract the type and value automatically.
But that will still mean changes to
the existing callers, which doesn't
make it any better than not doing
anything.
Четверг, 9 июля 2026 г получено от Andrew Bird:
… andrewbird left a comment (dosemu2/fdpp#299)
I made a draft PR that seems to work #300, but I wonder if there's some way that _sprintf() could do the GET_PTR/GET_FP32 translation instead of them being splattered all over the source?
--
Reply to this email directly or view it on GitHub:
#299 (comment)
You are receiving this because you commented.
Message ID: ***@***.***
|
|
Google gave me some hints on what to experiment with to implement GET_PTR() / GET_FP32() conversions outside of the kernel (for a later PR if successful). If I want to implement _sprintf() etc as C++ using templates and concepts I guess I'll need to put it in a separate .cpp file somewhere. Would that live in fdpp directory alongside farptr.*? |
|
Yes, you can put it into fdpp, |
|
I don't think it will work either, certainly not in a portable fashion. |
|
Hmm... Actually there is a way. |
|
And I really think that's worth doing, |
|
My clang-tidy (v18) on Noble doesn't seem to have that option, so I guess it can't be part of the build yet. |
|
Are you sure? |
|
I was expecting to see the option when the usage as displayed. However doing |
|
I woke up this morning and wondered if we could process the Dos C files with coccinelle to do the replacements. But to be honest I don't really understand the syntax and was pretty sure it couldn't handle variadic replacements. Any way Google's suggestion was to process it with python using tree sitter. That turned out to be non starter, but plain python seems to work okay. How do you feel about AI generated stuff? I have it converting a file Anyway, here's a sample of its output $ python3 fix_printf10.py mytest.c mytest.cpp
$ diff -surN mytest.c mytest.cpp
--- mytest.c 2026-07-10 10:18:51.408344281 +0100
+++ mytest.cpp 2026-07-10 10:41:53.024348288 +0100
@@ -539,19 +539,19 @@
sftp->sft_attrib = attrib = attrib | D_ARCHIVE;
/* check for a (local) device */
- DebugPrintf(("result for %Fs is %s%s%s%s",fname,(result & IS_DEVICE)?"device ":"X",(result & IS_NETWORK)?"network ":"N",(dhp = IsDevice(fname)) != NULL?"IsDevice!=NULL ":"="));
+ DebugPrintf(("result for %s is %s%s%s%s",GET_PTR(fname),(result & IS_DEVICE)?"device ":"X",(result & IS_NETWORK)?"network ":"N",(dhp = IsDevice(fname)) != NULL?"IsDevice!=NULL ":"="));
/* same line just different formatting */
- DebugPrintf(("result for %Fs is %s%s%s%s",
- fname,
+ DebugPrintf(("result for %s is %s%s%s%s",
+ GET_PTR(fname),
(result & IS_DEVICE)?"device ":"X",
(result & IS_NETWORK)?"network ":"N",
(dhp = IsDevice(fname)) != NULL?"IsDevice!=NULL ":"="));
/* another line with %P */
- DebugPrintf(("result for %Fs is %P\n",
- fname,
- fname));
+ DebugPrintf(("result for %s is %P\n",
+ GET_PTR(fname),
+ GET_FP32(fname)));
if ((result & IS_DEVICE) && !(result & IS_NETWORK) && (dhp = IsDevice(fname)) != NULL)
{
@@ -1004,7 +1004,7 @@
COUNT result;
#if defined(CHDIR_DEBUG)
- DebugPrintf(("DosChangeDir: %Fs\n", s));
+ DebugPrintf(("DosChangeDir: %s\n", GET_PTR(s)));
#endif
result = truename(s, PriPathName, CDS_MODE_CHECK_DEV_PATH);
if (result < SUCCESS)
@@ -1022,14 +1022,14 @@
return DE_PATHNOTFND;
#if defined(CHDIR_DEBUG)
- DebugPrintf(("Remote Chdir: n='%Fs' p='%s\n", s, PriPathName));
+ DebugPrintf(("Remote Chdir: n='%s' p='%s\n", GET_PTR(s), PriPathName));
#endif
/* now get fs to change to new */
/* directory */
result = (result & IS_NETWORK ? network_redirector(REM_CHDIR) :
dos_cd(PriPathName));
#if defined(CHDIR_DEBUG)
- DebugPrintf(("status = %04x, new_path='%Fs'\n", result, current_ldt->cdsCurrentPath));
+ DebugPrintf(("status = %04x, new_path='%s'\n", result, GET_PTR(current_ldt->cdsCurrentPath)));
#endif
/* on error result should be negative, on success for network call value
should be ignored as some redirectors e.g. Lantastic return count on success */
@@ -1071,7 +1071,7 @@
int rc;
register dmatch FAR *dmp = dta;
- DebugPrintf(("DosFindFirst(%Fs, 0x%04x)\n", name, attr));
+ DebugPrintf(("DosFindFirst(%s, 0x%04x)\n", GET_PTR(name), attr));
rc = truename(name, PriPathName,
CDS_MODE_CHECK_DEV_PATH | CDS_MODE_ALLOW_WILDCARDS);
if (rc < SUCCESS)
@@ -1090,7 +1090,7 @@
SAttr = (BYTE) attr;
#if defined(FIND_DEBUG)
- DebugPrintf(("Remote Find: n='%Fs\n", PriPathName));
+ DebugPrintf(("Remote Find: n='%s\n", GET_PTR(PriPathName)));
#endif
dta = &sda_tmp_dm;And the python import sys
import re
# Combined transformation configuration map
TRANSFORMS_MAP = {
'Fs': {
'wrapper': 'GET_PTR',
'replacement': 's'
},
'P': {
'wrapper': 'GET_FP32',
'replacement': None # Change to a string if you need to mutate %P -> %p
}
}
# Borland + Standard Function Index Map
FORMAT_INDEX_MAP = {
'printf': 0, 'fprintf': 1, 'sprintf': 1, 'snprintf': 2,
'cprintf': 0, 'wprintf': 0, 'fwprintf': 1, 'swprintf': 1,
'DebugPrintf': 0
}
FUNC_PATTERN = re.compile(rf"\b({'|'.join(FORMAT_INDEX_MAP.keys())})\s*\(", re.MULTILINE)
def extract_balanced_args_with_positions(text, start_pos):
"""
Steps through characters starting at an open parenthesis.
Returns a list of tuples containing: (arg_text, global_start_idx, global_end_idx).
Preserves all internal whitespace, tabs, and newlines exactly.
"""
args = []
current_arg = []
paren_depth = 0
in_string = False
escape = False
arg_start = start_pos + 1
i = start_pos
while i < len(text):
char = text[i]
if in_string:
current_arg.append(char)
if escape:
escape = False
elif char == '\\':
escape = True
elif char == '"':
in_string = False
else:
if char == '"':
in_string = True
current_arg.append(char)
elif char == '(':
paren_depth += 1
if paren_depth > 1:
current_arg.append(char)
elif char == ')':
paren_depth -= 1
if paren_depth == 0:
if current_arg or len(args) > 0:
args.append(("".join(current_arg), arg_start, i))
return i + 1, args
current_arg.append(char)
elif char == ',' and paren_depth == 1:
args.append(("".join(current_arg), arg_start, i))
current_arg = []
arg_start = i + 1
else:
current_arg.append(char)
i += 1
return -1, []
def process_file(input_path, output_path):
with open(input_path, 'r', encoding='utf-8', errors='ignore') as f:
source_code = f.read()
has_errors = False
modifications = []
pos = 0
while True:
match = FUNC_PATTERN.search(source_code, pos)
if not match:
break
func_name = match.group(1)
func_start = match.start()
open_paren_pos = match.end() - 1
end_pos, args = extract_balanced_args_with_positions(source_code, open_paren_pos)
if end_pos == -1 or not args:
pos = match.end()
continue
# Double-parentheses macro unpacking layer
if len(args) == 1 and args[0][0].strip().startswith('(') and args[0][0].strip().endswith(')'):
inner_text_raw, inner_start, inner_end = args[0]
first_paren = inner_text_raw.find('(')
_, inner_args = extract_balanced_args_with_positions(inner_text_raw, first_paren)
if inner_args:
args = [(txt, inner_start + s, inner_start + e) for txt, s, e in inner_args]
fmt_index = FORMAT_INDEX_MAP[func_name]
if fmt_index >= len(args):
pos = end_pos
continue
fmt_arg_text, fmt_start, fmt_end = args[fmt_index]
fmt_arg_stripped = fmt_arg_text.strip()
if not (fmt_arg_stripped.startswith('"') and fmt_arg_stripped.endswith('"')):
pos = end_pos
continue
fmt_str = fmt_arg_stripped[1:-1]
specifier_pattern = re.compile(r'%(?:%|[0-9.+\-*#lhzj]*[a-zA-Z]+)')
specifiers = [t for t in specifier_pattern.findall(fmt_str) if t != '%%']
vargs = args[fmt_index + 1:]
updated_fmt_str = fmt_str
format_string_modified = False
for i, spec in enumerate(specifiers):
if i >= len(vargs):
break
arg_text, arg_start, arg_end = vargs[i]
arg_stripped = arg_text.strip()
# Identify if this specifier matches any configured token keys
target_token = next((token for token in TRANSFORMS_MAP if token in spec), None)
if target_token:
# ENFORCED POLICY ERROR: Fail build if un-casted macros or functions sit in the target area
if '(' in arg_stripped and not re.match(r'^\s*\([^)]+\)\s*[a-zA-Z_]', arg_stripped):
line_no = source_code.count('\n', 0, func_start) + 1
print(f"Error: [{input_path}:{line_no}] Found restricted macro/function call '{arg_stripped}' inside {func_name}.", file=sys.stderr)
has_errors = True
continue
transform_config = TRANSFORMS_MAP[target_token]
wrapper = transform_config['wrapper']
replacement = transform_config['replacement']
# Wrap the text chunk while safely preserving leading and trailing spaces
leading_spaces = arg_text[:len(arg_text)-len(arg_text.lstrip())]
trailing_spaces = arg_text[len(arg_text.rstrip()):]
wrapped_arg_text = f"{leading_spaces}{wrapper}({arg_stripped}){trailing_spaces}"
modifications.append((arg_start, arg_end, wrapped_arg_text))
# Check if the string literal specifier itself needs editing
if replacement is not None:
replaced_spec = spec.replace(target_token, replacement)
updated_fmt_str = updated_fmt_str.replace(spec, replaced_spec, 1)
format_string_modified = True
if format_string_modified and not has_errors:
leading_fmt_spaces = fmt_arg_text[:len(fmt_arg_text)-len(fmt_arg_text.lstrip())]
trailing_fmt_spaces = fmt_arg_text[len(fmt_arg_text.rstrip()):]
new_fmt_arg_text = f'{leading_fmt_spaces}"{updated_fmt_str}"{trailing_fmt_spaces}'
modifications.append((fmt_start, fmt_end, new_fmt_arg_text))
pos = end_pos
if has_errors:
sys.exit(1)
modifications.sort(key=lambda x: x[0], reverse=True)
mutable_source = list(source_code)
for start, end, new_text in modifications:
mutable_source[start:end] = list(new_text)
with open(output_path, 'w', encoding='utf-8') as f:
f.write("".join(mutable_source))
if __name__ == "__main__":
if len(sys.argv) < 3:
print("Usage: python fix_printf.py <input.c> <output.cpp>")
sys.exit(1)
process_file(sys.argv[1], sys.argv[2]) |
|
So why not to use clang-tidy, why |
|
... why it can't be applied, correction. |
|
Sure, it doesn't look particularly frightening to review, and it looks like it will achieve the goal. I know I don't write the toolchain tools, so I shouldn't be bothered, but AI gifted code is unsatisfying especially now it's getting better. I guess I should stop asking it questions... I'll investigate clang-tidy though first. |
I don't understand what you say. |
|
Yes, asking AI questions is how these things get started for me. If I were talking to a human reviewer I wouldn't expect or desire for an initial code snippet to get fleshed out into a working solution. I need to learn to stop asking follow up questions, and if that code snippet doesn't compile, just accept it, figure out what and fix it myself. I think it can be laziness on my part, what the AI doesn't understand is that for me the goal is not the fix but the process. Handing me a turnkey solution may solve the task at hand, but is not enjoyable for me and in the end the goal was not met. I know this is coming across as an ungrateful rant, I'm sorry that's the case, and I'm sure it's a 'me' problem. |
The problem with software development, |
2 participants
1/ Some changes to make the debugging to dosemu logging compile after enabling lots of DEBUG_* defines.
2/ Tidy up some of the messages now that the logging function prefixes every write.
3/ Tweak the CI to collect and archive any failure logs that may occur.