Skip to content

[No ticket] Xcode 26 quick fixes#89

Merged
pastey merged 8 commits intomainfrom
no-ticket/xcode-26-quick-fix
Sep 30, 2025
Merged

[No ticket] Xcode 26 quick fixes#89
pastey merged 8 commits intomainfrom
no-ticket/xcode-26-quick-fix

Conversation

@r-markhyvka
Copy link

@r-markhyvka r-markhyvka commented Sep 29, 2025

When building with Xcode 26, there were two issues:

  1. Passing 'printf' format string where 'NSString' format string is expected

Where: S7Logging.m

Why: Xcode oh so kindly warns us, that, you see, if you pass a printf format string to where an NSString format string is expected bad things could happen like incorrect output, security vulnerabilities (!) and even crashes (c) ChatGPT
So it's safer to wrap it up in @"%@" format;

  1. Variable length array folded to constant array as an extension

Where: S7ConfigMergeDriver.m

Why: This error says that "" is not specific - or not constant - enough and when considering a code portability, other compilers or different compilation settings might treat const unsigned int N differently, leading to errors or unexpected behavior. Defining it with macro instead, "seals" the constant-ity of the array and thus - no more complaints :)


Note

Bumps macOS deployment target to 14.6, rewrites logging to C-style printf formatting, adds a fopen NULL check in packed-refs parsing, and replaces a const buffer size with a macro in the merge driver.

  • Build:
    • Bump MACOSX_DEPLOYMENT_TARGET to 14.6 in system7.xcodeproj/project.pbxproj.
  • Utils/Logging (system7/Utils/S7Logging.m):
    • Implement C-style formatting via vsnprintf in new formatMessage and use it in logInfo/logError (avoid NSString formatting); free allocated buffers.
  • Git (system7/git/Git.m):
    • Add NULL check and error logging when opening .git/packed-refs in findPackedReferenceMatchingPattern.
  • Merge Driver (system7/Merge Driver/S7ConfigMergeDriver.m):
    • Replace const int BUF_LEN with #define BUF_LEN for stdin buffer size.

Written by Cursor Bugbot for commit bc6816e. This will update automatically on new commits. Configure here.

va_list va_args;
va_start(va_args, format);
NSString *const nsFormat = [[NSString alloc] initWithCString:format encoding:NSUTF8StringEncoding];
NSString *const nsFormat = [NSString stringWithFormat:@"%@", [NSString stringWithUTF8String:format]];
Copy link
Author

@r-markhyvka r-markhyvka Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could write it even simpler with [NSString stringWithFormat:@"%s", format]; but I am not sure if we are prone to loose some of the characters, if we omit specifying the utf8 encoding.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we haven't actually fixed the compliers complain, but piled one more level of indirection, so it cannot find an issue anymore.
Would suggest to use vfprintf here. And there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pastey not sure I understand what you mean.
Do you mean, replacing the fprintf that we already have with vfprintf instead? It will still require the va_start/va_end, so it will be something like:

void logInfo(const char * __restrict format, ...) {
    va_list va_args;

    va_start(va_args, format);
    withTTYLockDo(^{
        vfprintf(stdout, format, va_args);
    });
    va_end(va_args);
}

but would that work with withTTYLockDo dispatch? My c++ knowledge is not extensive enough to know for sure :)

Alternatively, we could "prepare" the string with vsprintf first and then frpintf it as we do it now:

    va_list va_args;
    char message[256];

    va_start (va_args, format);
    vsprintf (message,format, va_args);
    va_end (va_args);

    char *messagePointer = message;
    withTTYLockDo(^{
        fprintf(stdout, "%s", messagePointer);
    });

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, the second logError function adds additional characters to the output when fprintf-ing it, so the "preparation" with vsprintf of the string seems to be the only way to go in that case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, vsprintf is a good option for this task 👍

@r-markhyvka r-markhyvka requested a review from pastey September 29, 2025 15:41
@r-markhyvka r-markhyvka self-assigned this Sep 29, 2025
va_list va_args;
va_start(va_args, format);
NSString *const nsFormat = [[NSString alloc] initWithCString:format encoding:NSUTF8StringEncoding];
NSString *const nsFormat = [NSString stringWithFormat:@"%@", [NSString stringWithUTF8String:format]];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we haven't actually fixed the compliers complain, but piled one more level of indirection, so it cannot find an issue anymore.
Would suggest to use vfprintf here. And there.

@r-markhyvka r-markhyvka requested a review from pastey September 30, 2025 14:32
@r-markhyvka
Copy link
Author

@pastey some of the tests failed with the new changes, making this "quick fix" not so quick anymore :(
Maybe you could spot what I did wrong to fix it right away?

If not, then how about I revert back to "shushing" Xcode 26 with the NSLog wrap, so we could merge it to make system7 usable on machines with Xcode 26, and then someone else could return to fixing it properly later?

Also, fyi, I just checked the main branch and the tests there are green, which means the CI is running old Xcode version and we'll be getting false-positives in the future.

… macOS >= 10.14. If someone need s7 on earlier versions, we'll ask them to tweak the setting on their machine
@pastey pastey enabled auto-merge September 30, 2025 19:58
@pastey pastey disabled auto-merge September 30, 2025 20:03
@pastey pastey enabled auto-merge September 30, 2025 20:03
@pastey pastey merged commit e8f962f into main Sep 30, 2025
3 checks passed
@pastey pastey deleted the no-ticket/xcode-26-quick-fix branch September 30, 2025 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants