Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix arc4 compilation debian #343

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

endaco
Copy link

@endaco endaco commented Jun 6, 2024

Purpose

This Pull Request addresses and fixes various compilation issues on Debian 12 systems related to ARC4 and time/date functions

Changes Made

  • arc4.c:

    • Applied corrections to ensure compilation compatibility with GCC on Debian 12.
  • settime.c:

    • Adjustments made to improve precision in setting the system time.
  • setdate.c:

    • Updated the logic for setting the dates.
  • dattime3.c:

    • Improved date and time handling.
  • firebird.c:

    • Modified the connection routine to use hb_snprintf to prevent possible buffer overflow issues.

Impact

These changes ensure that the code compiles correctly and operates smoothly on Debian 12 systems.

Tests Conducted

  • Successfully compiled the project on Debian 12.
  • Ran existing unit tests to ensure no regressions.
  • Conducted functional tests to validate changes in time and date functions and integration with Firebird.

endaco added 7 commits June 5, 2024 10:26
@vszakats vszakats force-pushed the main branch 2 times, most recently from fcf9b65 to a22b0ea Compare January 23, 2025 03:43
@@ -149,41 +149,42 @@ HB_FUNC( FBCREATEDB )
hb_retnl( 0 );
}

HB_FUNC( FBCONNECT )
HB_FUNC(FBCONNECT)
Copy link
Owner

Choose a reason for hiding this comment

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

It'd make it easier to review and merge without formatting/comment changes to existing code.

len = (int)(sizeof(dpb) - i - 2);
i += hb_snprintf(dpb + i, sizeof(dpb) - i, "%c", (char)len);
hb_strncpy(&(dpb[i]), passwd, len);
i += (short)len;
Copy link
Owner

Choose a reason for hiding this comment

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

Can the above hb_snprintf() calls be made fewer or into a single one?

Copy link
Owner

Choose a reason for hiding this comment

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

Pushed a fix for this that uses a single call. Untested.

@@ -1,4 +1,5 @@
/* Rewritten in 2012 by Viktor Szakats and kept in the
/*
* Rewritten in 2012 by Viktor Szakats and kept in the
Copy link
Owner

Choose a reason for hiding this comment

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

Unrelated change.

@@ -1,4 +1,5 @@
/* Rewritten in 2012 by Viktor Szakats and kept in the
/*
* Rewritten in 2012 by Viktor Szakats and kept in the
Copy link
Owner

Choose a reason for hiding this comment

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

Unrelated change.

cLangOld := hb_langSelect( "en" ) /* to always have RTEs in one language */


cLangOld := __hb_langSelect( "en" ) /* to always have RTEs in one language */
Copy link
Owner

Choose a reason for hiding this comment

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

Why doing this change?

}




Copy link
Owner

Choose a reason for hiding this comment

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

nit: Missing standard indentation and extra lines.

ts.tv_sec = mktime(tm_info);
ts.tv_nsec = 0;

fResult = clock_settime(CLOCK_REALTIME, &ts) == 0;
Copy link
Owner

Choose a reason for hiding this comment

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

I pushed a fixed for this.

ts.tv_sec = mktime(tm_info);
ts.tv_nsec = 0;

fResult = clock_settime(CLOCK_REALTIME, &ts) == 0;
Copy link
Owner

Choose a reason for hiding this comment

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

I have a patch for this, but it doesn't apply, not sure why.

vszakats added a commit that referenced this pull request Jan 23, 2025
Limited build-test only.

Ref: #343
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.

2 participants