Ticket #180 (closed defect: fixed)

Opened 7 years ago

Last modified 6 years ago

(mandriva) concat_dir_and_file() + NULL parameter fix

Reported by: metux Owned by: metux
Priority: major Milestone: 4.6.2
Component: mc-core Version: 4.6.1
Keywords: committed-mc-4.6 committed-master Cc:
Blocked By: Blocking:
Branch state: Votes for changeset:

Description (last modified by metux) (diff)

Attachments

patch-vfs.patch (7.2 KB) - added by styx 7 years ago.
180_concat_dir_and_null.patch (1.8 KB) - added by styx 7 years ago.
sorry, I'm messed up! this one should be

Change History

Changed 7 years ago by styx

Changed 7 years ago by styx

sorry, I'm messed up! this one should be

comment:1 Changed 7 years ago by styx

  • Keywords vote-styx added

just move patch here and make a review

comment:2 Changed 7 years ago by winnie

  • Milestone changed from 4.7 to 4.6.3

comment:3 in reply to: ↑ description Changed 7 years ago by guanx

Replying to metux:

http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/mc/current/SOURCES/mc-concat.patch?view=markup

Here: const char *file = filename;
Why a separate variable for the filename pointer?

And, in the patch, there is this comment:
/* Is this one: *file == PATH_SEP seems to be not fully portable? */
It's also confusing.

comment:4 follow-up: ↓ 8 Changed 7 years ago by metux

  • Owner set to metux
  • Status changed from new to accepted
  • Description modified (diff)

I've reworked the whole function and replaced it by mhl_str_dir_plus_file() in mhl/string.h

comment:5 Changed 7 years ago by slyfox

  • Keywords vote-slyfox approved added; review removed

comment:6 Changed 7 years ago by metux

  • Keywords committed-mc-4.6 committed-master added; mandriva vote-styx vote-slyfox approved removed
  • Status changed from accepted to testing
  • Resolution set to fixed
  • Description modified (diff)

comment:7 Changed 7 years ago by winnie

  • Status changed from testing to closed
  • Milestone changed from 4.6.3 to 4.6.2

Moving this to 4.6.2 as this is already fixed in 4.6 branch. We need to update NEWS file.

comment:8 in reply to: ↑ 4 Changed 7 years ago by andrew_b

  • Status changed from closed to reopened
  • Resolution fixed deleted

Replying to metux:

I've reworked the whole function and replaced it by mhl_str_dir_plus_file() in mhl/string.h

First:

charsets.o: In function `load_codepages_list':
/home/andrew/work.c/mc/mc.master/src/charsets.c:51: undefined reference to `mhl_str_dir_plus_file'
../vfs/libvfs-mc.a(smbfs.o): In function `smbfs_get_path':
/home/andrew/work.c/mc/mc.master/vfs/smbfs.c:1214: undefined reference to `mhl_str_dir_plus_file'
collect2: ld returned 1 exit status

#include <mhl/string.h> is forgotten in those files.

Second: in mhl_str_dir_plus_file() there are incorrect type of strlen() results: int instead of size_t.

comment:9 Changed 7 years ago by metux

  • Status changed from reopened to accepted

comment:10 Changed 7 years ago by metux

  • Status changed from accepted to testing
  • Resolution set to fixed

comment:11 Changed 7 years ago by slyfox

charsets.o: In function `load_codepages_list':
/home/andrew/work.c/mc/mc.master/src/charsets.c:51: undefined reference to `mhl_str_dir_plus_file'
../vfs/libvfs-mc.a(smbfs.o): In function `smbfs_get_path':
/home/andrew/work.c/mc/mc.master/vfs/smbfs.c:1214: undefined reference to `mhl_str_dir_plus_file'
collect2: ld returned 1 exit status

Those are fixed and will be available in mc-4.6.2.
Thanks!

Second: in mhl_str_dir_plus_file() there are incorrect type of strlen() results: int instead of size_t.

It is sane cleanup. Will be applied in next (after mc-4.6.2) bugfix.
Thanks again! :]

comment:12 Changed 7 years ago by winnie

  • Status changed from testing to closed

Okay.. I'll close this report.. the type cleanup will be available in the next version of mc (4.6.3 or 4.6.2.1, not sure up to now).

Note: See TracTickets for help on using tickets.