On 27/04/13 10:00, Andrew Gregory wrote:
> Signed-off-by: Andrew Gregory <[email protected]>
> ---
> lib/libalpm/add.c | 33 ++++++++-------------------------
> 1 file changed, 8 insertions(+), 25 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 1d9db60..0b8aedf 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -204,19 +204,17 @@ static int extract_single_file(alpm_handle_t *handle,
> struct archive *archive,
> * D | 10 | 11 | 12
> *
> * 1,2,3- extract, no magic necessary. lstat (_alpm_lstat) will fail
> here.
> - * 4,5,6,7,8- conflict checks should have caught this. either overwrite
> + * 4,5,6,7,8,9- conflict checks should have caught this. either
> overwrite
> * or backup the file.
> - * 9- follow the symlink, hopefully it is a directory, check it.
> - * 10- file replacing directory- don't allow it.
> - * 11- don't extract symlink- a dir exists here. we don't want links to
> - * links, etc.
> + * 10,11- file replacing directory- don't allow it.
> * 12- skip extraction, dir already exists.
> */
>
> - /* do both a lstat and a stat, so we can see what symlinks point to */
> struct stat lsbuf, sbuf;
> - if(_alpm_lstat(filename, &lsbuf) != 0 || stat(filename, &sbuf) != 0) {
> - /* cases 1,2,3: couldn't stat an existing file, skip all backup
> checks */
> + if(stat(filename, &sbuf) != 0 || _alpm_lstat(filename, &lsbuf) != 0) {
> + /* cases 1,2,3: file doesn't exist or is a broken symlink,
> + * skip all backup checks */
> + /* TODO: should we really treat broken symlinks as if they
> don't exist? */
NO! We definitely should not...
In fact, I'd like to see this patch result in this:
* (F=file, N=node, S=symlink, D=dir)
* | F/N/S | D
* non-existent | 1 | 2
* F/N/S | 3 | 4
* D | 5 | 6
because now there is no difference between a file and a symlink. Which
probably means getting rid of the lstat call (although I did not look at
the code to confirm...)
> } else {
> if(S_ISDIR(lsbuf.st_mode)) {
> if(S_ISDIR(entrymode)) {
> @@ -243,23 +241,8 @@ static int extract_single_file(alpm_handle_t *handle,
> struct archive *archive,
> archive_read_data_skip(archive);
> return 1;
> }
> - } else if(S_ISLNK(lsbuf.st_mode) && S_ISDIR(entrymode)) {
> - /* case 9: existing symlink, dir in package */
> - if(S_ISDIR(sbuf.st_mode)) {
> - /* the symlink on FS is to a directory, so
> we'll use it */
> - _alpm_log(handle, ALPM_LOG_DEBUG, "extract:
> skipping symlink overwrite of %s\n",
> - filename);
> - archive_read_data_skip(archive);
> - return 0;
> - } else {
> - /* this is BAD. symlink was not to a directory
> */
> - _alpm_log(handle, ALPM_LOG_ERROR, _("extract:
> symlink %s does not point to dir\n"),
> - filename);
> - archive_read_data_skip(archive);
> - return 1;
> - }
> - } else if(S_ISREG(lsbuf.st_mode) && S_ISDIR(entrymode)) {
> - /* case 6: trying to overwrite file with dir */
> + } else if(!S_ISDIR(lsbuf.st_mode) && S_ISDIR(entrymode)) {
> + /* case 6,9: trying to overwrite file with dir */
> _alpm_log(handle, ALPM_LOG_DEBUG, "extract: overwriting
> file with dir %s\n",
> filename);
> } else if(S_ISREG(entrymode)) {
>
