Re: Deleting a line from a file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 14 May 2014 22:13:51 +0530, Saket Sinha said:

> I am sending /etc/fstab in fileName to this function and the path to
> be deleted in fullPath

OK.

>  char newFileName[PATH_MAX];

This lives on your function call stack. As such, it contains whatever
was in that memory until you change it.  And you never change it.

     tabFileOld = setmntent( fileName, "r" ); // Open for writing now

The comment indicates there's some confusion going on. Not a good sign....

     tabFileNew = setmntent(newFileName, "w");
     if (tabFileNew == NULL )
           goto end;

This probably doesn't do what you think it does.  What value of 'newFileName'
is used to create the new file?  For bonus points, what guarantees that
it points to an object that's on the same filesystem as your old filename?

(This will actually fail to throw an error as long as newFileName[0] isn't
a '\0'.  But that's different from actually working properly....)

And you *really* want to get this issue well-understood before hacking kernel
code, as the kernel doesn't do very much error checking.  You make this sort of
error in kernel code, you will crash the machine - if you're lucky.  If you're
unlucky, the system will silently corrupt entire filesystems on you to the
point where fsck won't help.  (Want *real* debugging fun?  Get your system
to a state where /boot fsck's just fine, but something has silently overwritten
several data blocks in /boot/vmlinuz or /lib/ld-linux.so.  *That* will keep
you busy for a while.. :)

          if (tcscmp(MY_FS_TYPE, m->mnt_type) == 0)

You want to use strncmp() or similar here.  And you want to learn why
you shouldn't be using tcscmp() (Hint: are either MY_FS_TYPE or m->mnt_type
allowed to be UTF-8 strings? :)

     rename(newFileName, fileName))
     rc = 0;

You probaby want to check the return code:

	rc = rename(newFileName, fileName);
	if (!rc) { perror() or something....

as you're *very* likely to get EXDEV as an error due to previous code.

You're also missing error checking on every single endmntent() call - what
happens if one of those fails (which *can* happen)?

	sync();

You don't want to do that.  You should have done an fsync() (except that's
technically wrong as you're mixing it with stdio and not setting it to
unbuffered) or a syncfs() instead.

	return rc;

This fails to capture a number of potential errors.

Attachment: pgp_n9HCr3dQo.pgp
Description: PGP signature

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux