Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Comparison unsigned expression less then zero #482

Closed
Milestone
@japinli

Description

@japinli

Hi,

I find there are some unnecessary comparison for unsigned type, for example

size_tpos=ftell(out);if (pos<0)elog(ERROR,"Cannot get position in destination file \"%s\": %s",to_fullpath,strerror(errno));

Since thepos issize_t, which is unsigned, so the following if condition alway false.
I checked that theftell() returnslong, which is singed.

Thedo_delete() anddo_delete_status() have the same problems. we declaration
thesize_to_delete to unsigned, however, the check condition always return true.

Here is a patch for those.

diff --git a/src/data.c b/src/data.cindex f02e3fd1..ec42813a 100644--- a/src/data.c+++ b/src/data.c@@ -2321,7 +2321,7 @@ copy_pages(const char *to_fullpath, const char *from_fullpath, elog(ERROR, "Cannot seek to end of file position in destination file \"%s\": %s",  to_fullpath, strerror(errno)); {-size_t pos = ftell(out);+long pos = ftell(out);  if (pos < 0) elog(ERROR, "Cannot get position in destination file \"%s\": %s",diff --git a/src/delete.c b/src/delete.cindex 6c70ff81..6335482f 100644--- a/src/delete.c+++ b/src/delete.c@@ -36,7 +36,7 @@ do_delete(InstanceState *instanceState, time_t backup_id) parray   *backup_list,    *delete_list; pgBackup   *target_backup = NULL;-size_tsize_to_delete = 0;+int64size_to_delete = 0; charsize_to_delete_pretty[20];  /* Get complete list of backups */@@ -1027,7 +1027,7 @@ do_delete_status(InstanceState *instanceState, InstanceConfig *instance_config, parray     *backup_list, *delete_list; const char *pretty_status; int         n_deleted = 0, n_found = 0;-size_t      size_to_delete = 0;+int64       size_to_delete = 0; char        size_to_delete_pretty[20]; pgBackup   *backup;

instance_config.wal_depth >= 0 also always true, sincewal_depth is unsigned. Should we remove the condition check, or change it to signed?

Here is a patch to fix this by removing the condition check.

diff --git a/src/delete.c b/src/delete.cindex 6c70ff81..9467c8dc 100644--- a/src/delete.c+++ b/src/delete.c@@ -682,12 +682,11 @@ do_retention_wal(InstanceState *instanceState, bool dry_run)                 * at least one backup and no file should be removed.                 * Unless wal-depth is enabled.                 */-               if ((tlinfo->closest_backup) && instance_config.wal_depth <= 0)+               if ((tlinfo->closest_backup) && instance_config.wal_depth == 0)                        continue;                /* WAL retention keeps this timeline from purge */-               if (instance_config.wal_depth >= 0 && tlinfo->anchor_tli > 0 &&-                       tlinfo->anchor_tli != tlinfo->tli)+               if (tlinfo->anchor_tli > 0 && tlinfo->anchor_tli != tlinfo->tli)                        continue;                /*@@ -701,7 +700,7 @@ do_retention_wal(InstanceState *instanceState, bool dry_run)                 */                if (tlinfo->oldest_backup)                {-                       if (instance_config.wal_depth >= 0 && !(XLogRecPtrIsInvalid(tlinfo->anchor_lsn)))+                       if (!(XLogRecPtrIsInvalid(tlinfo->anchor_lsn)))                        {                                delete_walfiles_in_tli(instanceState, tlinfo->anchor_lsn,                                                                tlinfo, instance_config.xlog_seg_size, dry_run);@@ -714,7 +713,7 @@ do_retention_wal(InstanceState *instanceState, bool dry_run)                }                else                {-                       if (instance_config.wal_depth >= 0 && !(XLogRecPtrIsInvalid(tlinfo->anchor_lsn)))+                       if (!(XLogRecPtrIsInvalid(tlinfo->anchor_lsn)))                                delete_walfiles_in_tli(instanceState, tlinfo->anchor_lsn,                                                                tlinfo, instance_config.xlog_seg_size, dry_run);                        else@@ -942,7 +941,7 @@ delete_walfiles_in_tli(InstanceState *instanceState, XLogRecPtr keep_lsn, timeli                        join_path_components(wal_fullpath, instanceState->instance_wal_subdir_path, wal_file->file.name);                        /* save segment from purging */-                       if (instance_config.wal_depth >= 0 && wal_file->keep)+                       if (wal_file->keep)                        {                                elog(VERBOSE, "Retain WAL segment \"%s\"", wal_fullpath);                                continue;

Any suggestions?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


      [8]ページ先頭

      ©2009-2025 Movatter.jp