Ticket #180 (closed defect: fixed)

Opened 10 years ago

Last modified 9 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 10 years ago.
180_concat_dir_and_null.patch (1.8 KB) - added by styx 10 years ago.
sorry, I'm messed up! this one should be

Change History

Changed 10 years ago by styx

Changed 10 years ago by styx

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

comment:1 Changed 10 years ago by styx

  • Keywords vote-styx added

just move patch here and make a review

comment:2 Changed 10 years ago by winnie

  • Milestone changed from 4.7 to 4.6.3

comment:3 in reply to: ↑ description Changed 10 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 10 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 10 years ago by slyfox

  • Keywords vote-slyfox approved added; review removed

comment:6 Changed 10 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 10 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 10 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 10 years ago by metux

  • Status changed from reopened to accepted

comment:10 Changed 10 years ago by metux

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

comment:11 Changed 10 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 10 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.