kano-toolset icon indicating copy to clipboard operation
kano-toolset copied to clipboard

the parameter to open in kano-launch

Open wweiradio opened this issue 8 years ago • 1 comments

    for(int i=0;i<10;i++){
      int count=snprintf(my_contfile, STRING_SIZE,"%s/%d.%d",contdir,pid,i);
      int cf;
      if(count==0 || count==STRING_SIZE) {
	kano_log_error("kano-launcher: string too long making container file [%s]\n",contdir);
      }
      cf=open(my_contfile,S_IWUSR | S_IRUSR | S_IXUSR);
      if(cf){
	made_contfile=1;
	close(cf);
	break;
      }
    }

From kano-launcher.c Line 216-228

Looks like the line cf=open(my_contfile,S_IWUSR | S_IRUSR | S_IXUSR); has problem

  1. should the O_CREAT | O_EXCL be provided?
  2. should check if (cf > 0), instead of if(cf) ?

wweiradio avatar Jun 25 '17 06:06 wweiradio

Hi @wweiradio,

Thanks for looking at this. Having reviewed what you've said, it looks like you are correct about the open()- it seems like we should want to fail if the file already exists so that the same file is not bound to multiple containers. It seems that the subsequent lines might cause it to fail anyway and the default behaviour of just running the command cause this problem to be unnoticed. My only concern is whether these files are cleaned up properly after execution.

You are absolutely right about the second point, it should definitely be

if (cf > 0) {
   ...
}

If you would like to submit a pull request for these problems then we would be very grateful.

Also tagging the original author, @Ealdwulf, to check that there isn't a good reason that these should not be changed.

tombettany avatar Jun 28 '17 15:06 tombettany